Skip to content

Update DEV_s5#89

Closed
phoman14 wants to merge 6 commits intoDEV_S5from
DEV
Closed

Update DEV_s5#89
phoman14 wants to merge 6 commits intoDEV_S5from
DEV

Conversation

@phoman14
Copy link
Copy Markdown
Member

No description provided.

phoman14 and others added 6 commits March 18, 2026 14:14
* UsingClaude added Compare_Cell_Pop from NIDAPjson. passed AI tests

* fixing ModScoreHelpers_function. Error with Roxygen Format

* Update Vignette

* Update Vignette for MS app

* Start CompareCelPop Vignette

* Add code for compareCellPopulations Vignette

* add Vis_CCPbar.png

* Render Visualizations.Rmd with pkgdown

* resize CCPbar figure for vignette

* Add CCPbox to Vignette

* Add description for Compare Cell populations

* edit CCP function description in vignette

* modify ViolinPlot function name and add standard output

* change violin plot name for tests violinPlot_mod -> violinPlot

* Build Vignettes with updates

* Modify _pkgdown.yml and /Vignevignettes/README.Rmd -> vignettes/Intro.Rmd to work with devtoools::check()

* Reverting Intro.Rmd to README.Rmd

* tried to troubleshoot running vignetts with devtools::check(). did not work so run devtools::check(vignettes = FALSE)

* rmeove Intro.Rmd after changed to README.Rmd

* add htmlttols for Vignettes

* LOCAL: tried to troubleshoot running vignetts with devtools::check(). did not work so run devtools::check(vignettes = FALSE)

* Modifications to pass unit tests

* update vignette to use README.Rmd not Intro.Rmd

* Potential fix for pull request finding

Outdated, NIDAP specific function options that don't apply to Packaged function anymore. metadata.table is extracted internally

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* added Vignette docs folder

* remvoed metadata.table from Compare_cell_Populations.R because no loger used (nidapism)

* Initial plan

* Add else-stop to getParamCCP for unknown data values

Co-authored-by: phoman14 <21298645+phoman14@users.noreply.github.com>

* Initial plan

* Add else branch to getParamCCP() with stop() for unknown datasets

Co-authored-by: phoman14 <21298645+phoman14@users.noreply.github.com>

* adding an explicit else that stop()s with a clear message listing the supported dataset keys.

* remove sptatstat.corre from DESCRIPTION

* removed htmltools from DESCRIPTION

* Deleted docs/reference/violinPlot_mod.html

* Potential fix for pull request finding

removing unused references

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

removed violinPlot_mod references

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Initial plan

* Fix trailing comma in knitr chunk header in SCWorkflow-Visualizations.Rmd

Co-authored-by: phoman14 <21298645+phoman14@users.noreply.github.com>

* Potential fix for pull request finding

Metadata column validation happens before the code normalizes column names (replacing . with _ at line 158). If a caller passes annotation.column/group.column using underscores while the Seurat metadata still has dots, this will error even though the function later renames the columns. Consider normalizing object@meta.data colnames and the *.column arguments before computing missing.cols.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

remove violinPlot_mod Reference

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Run Tests after copiolot modifications

* Potential fix for pull request finding

createAnnoTable() builds cntTble by cbind()-ing unique(AnnoCol) with table(...). Because the initial matrix has numeric rownames (1..n) and table() may not return values in the same order (and can omit missing levels), this can silently misalign counts with clusters and produce an incorrect output table. Consider constructing counts with table(SO@meta.data[[AnnoCol]], SO@meta.data[[GroupCol]]) (or xtabs) so row/column names stay aligned, then compute frequencies from that matrix.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: phoman14 <21298645+phoman14@users.noreply.github.com>
* Update Roxygen parmameters/descriptions for Dual_Labeling

* Modify unit Tests for updated list output

* add use.assay roxygen parameter

* update unit tests for Heatmap

* add cluster.identities.table roxygen description"

* make table for cluster.identities.table when running unit tests

* modify unit tests to use plots slot instead of plot

* fixed typo param mad.topNgenes.limitsSet

* Update Test for FilterQC

* clean up vignette quotation ussaage

* Modscore change to standard output

* colorByMarkerTable-clean up function parameter layout and coment out library load

* fix Samples Bug in Plot_Metadata

* plot Metadata rearange function parameters

* Plot Metadata test error plot->plots for output

* Standardize parameters so that _->. and fix typo in parameter

* standardize parameters for inst/shiny/ModuleScoreApp

* modify test to look for plots instead of plot

* colorByGene-resolved cant find Samples error

* minor error fixes to test files to run correctly

* remove explicetly loading library in function

* return standard output list data and plot

* removue redundant use of samples to us only samples.to.include

* Harmony make standard object list of object and plots

* Harmony remove uncessary print statements

* Add title and description from .json files for Annotate_Cell_Types.R, DEG_Gene_Expression_Markers.R,Dual_Labeling.R,Filter_QC.R,Filter_Seurat_Object_by_Metadata.R

* add example function calls to roxygen section. Standardize Roxygen format between all functions

* unifiy parameter descriptions with .json files

* update documentation

* minor changes to missing packages and test not using standard output structure

* fix Violin plot not loading tidyr

* create vignettes with previous updates to variables

* make interactive plot an option for AggregateCounts

* recreate vignette

* Color_by_Genes_Automatic.R and Compare_Cell_Populations.R still did not have standard output

* Update NAMESPACE

modify pivot_longer and str_split import

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update R/Violin_Plots_by_Metadata.R

update import->importFrom for pivot_longer

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update R/Filter_QC.R

Filter_QC.R In roxygen, square brackets [...] are interpreted as Rd links, which gets rendered as \\link{CCBR} / \\link{scRNA-seq} in the generated .Rd. Since those topics likely don't exist, this can create broken links and R CMD check warnings. Use plain text or parentheses instead (e.g., Filter Low Quality Cells (CCBR scRNA-seq)) to avoid generating invalid links.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Declare patchwork dependency and import plot_layout in Harmony.R (#85)

* Initial plan

* Fix patchwork::plot_layout unqualified usage in Harmony.R

Co-authored-by: phoman14 <21298645+phoman14@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NIDAP-Community/SCWorkflow/sessions/9de58a68-7e10-4304-87be-6efae85ad882

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: phoman14 <21298645+phoman14@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: phoman14 <21298645+phoman14@users.noreply.github.com>
* ci: add gha workflows from MOSuite

* ci: run on dev branches

* ci: force R version 4.1.3

* chore: delete pryr from deps

* chore: add dev deps

* ci(pkgdown): use custom installation

* chore: pin seurat & matrix

* chore: Matrix version with period instead of hyphen

* chore: fix pkg version format

* chore: remove duplicate suggests field

* chore: usethis::use_tidy_description()

* chore(docker): set R version 4.1.3

* ci: use docker container for R deps

* ci: consolidate test-coverage into check workflow

* ci: fix container image def

* ci: hard-code container image
* ci: add gha workflows from MOSuite

* ci: run on dev branches

* ci: force R version 4.1.3

* chore: delete pryr from deps

* chore: add dev deps

* ci(pkgdown): use custom installation

* chore: pin seurat & matrix

* chore: Matrix version with period instead of hyphen

* chore: fix pkg version format

* chore: remove duplicate suggests field

* chore: usethis::use_tidy_description()

* chore(docker): set R version 4.1.3

* ci(docker): add dockerfile from Rich

Co-authored-by: Rich Finney <finneyr@users.noreply.github.com>

* chore(docker): switch to use remotes::install_version instead of biowulf pkgs

* chore(docker): use heredoc syntax to reduce line length

* ci(docker): install local SCWorkflow

* ci(docker): install dev deps

* ci(docker): check that all deps installed

* chore: add rs413s Dockerfile with mamba installation (#88)

* ci(docker): try pinning mamba pkg versions based on rs413s docker

* ci(docker): remove unused scripts

* ci(docker): check all pkgs installed

* ci(docker): try pinning only the seurat version

---------

Co-authored-by: Rich Finney <finneyr@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates SCWorkflow’s plotting/template APIs and documentation, adds new reference/docs content (pkgdown outputs), and refreshes CI/Docker configuration aligned to an R 4.1.3 environment.

Changes:

  • Standardize many functions/tests around returning structured list(data=..., plots=...) outputs and updated argument naming (e.g., plots vs plot, *.by/*.var conventions).
  • Expand roxygen/Rd + pkgdown site outputs with new/updated examples, reference pages, and vignettes.
  • Update container/CI workflows (R version pin, pkgdown, R CMD check, coverage) and GitHub templates.

Reviewed changes

Copilot reviewed 146 out of 215 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
vignettes/images/Vis_3D_files/crosstalk-1.2.0/scss/crosstalk.scss Adds Crosstalk SCSS asset for vignette HTML output.
vignettes/images/Vis_3D_files/crosstalk-1.2.0/css/crosstalk.min.css Adds minified Crosstalk CSS asset for vignette HTML output.
vignettes/SCWorkflow-Usage.Rmd Fixes vignette index entry to match the vignette name.
vignettes/SCWorkflow-SubsetReclust.Rmd Normalizes quoting/style and adjusts spacing around chunks/images.
vignettes/SCWorkflow-QC.Rmd Normalizes quoting/boolean style and fixes parameter names in examples.
vignettes/README.Rmd Converts README vignette metadata to an html_vignette with proper index entry.
tests/testthat/test-Violin_Plots_by_Metadata.R Updates violin plot tests to new function name/return structure (plots).
tests/testthat/test-Recluster_Seurat_Object.R Updates expected output key from plot to plots.
tests/testthat/test-Plot_Metadata.R Updates expected output key from plot to plots.
tests/testthat/test-Name_Clusters_by_Enriched_Cell_Type.R Updates expected output keys and assertions for updated nameClusters output.
tests/testthat/test-ModuleScore.R Updates commented snapshot references from ms.figures to ms.plots and arg names.
tests/testthat/test-Heatmap.R Updates expected output key from plot to plots.
tests/testthat/test-Filter_Seurat_Object_by_Metadata.R Updates expected output keys to consolidated plots.
tests/testthat/test-Filter_QC.R Updates expected output keys (FilteringTablesdata) and table access paths.
tests/testthat/test-Dual_Labeling.R Updates expected output keys to data/plots and plot element names.
tests/testthat/test-Dotplot_by_Metadata.R Updates expected output structure to plots/data.
tests/testthat/test-DEG_Gene_Expression_Markers.R Updates expected output structure (dfdata).
tests/testthat/test-Color_by_Gene.R Updates expected output keys and plot indexing from plotplots.
tests/testthat/test-Annotate_Cell_Types.R Updates expected output keys to consolidated plots.
tests/testthat/test-AggregateCounts.R Updates tests to expect res$data output wrapper.
tests/testthat/test-3D_tSNE.R Updates expected output key from plot to plots.
tests/testthat/helper-Violin_Plots_by_Metadata.R Updates helper arg naming to dotted style and uses Seurat assay extraction for gene sampling.
tests/testthat/helper-Name_Clusters_by_Enriched_Cell_Type.R Adds cluster.identities.table helper and remaps inputs to table-column naming.
tests/testthat/helper-ModuleScore.R Renames args to ms.threshold/use.columns and updates returned parameter list.
tests/testthat/helper-Filter_QC.R Adds do.doublets.filter = TRUE to helper params.
tests/testthat/helper-Compare_Cell_Populations.R Adds new helper utilities for compareCellPopulations tests.
test_workflow_from_Harmony.R Updates argument names to dotted style for violin plot workflow script.
man/violinPlot_mod.Rd Removes obsolete documentation for violinPlot_mod.
man/violinPlot.Rd Adds new documentation for violinPlot.
man/tSNE3D.Rd Adds \value{} and examples section.
man/processRawData.Rd Adds examples section.
man/plotMetadata.Rd Adds examples section.
man/nameClusters.Rd Updates argument ordering/meaning and adds examples.
man/modscore-imports.Rd Adds internal helper doc for compute_modscore_data.
man/modscore-imports-011726.Rd Adds internal helper doc for _011726 variant.
man/modScore.Rd Renames args (use_columnsuse.columns, ms_thresholdms.threshold) and adds group.var.
man/launch_module_score_app.Rd Adds documentation for launching the ModuleScore Shiny app.
man/heatmapSC.Rd Renames use_assayuse.assay, clarifies row.order, adds examples.
man/harmonyBatchCorrect.Rd Fixes harmonyBatchCorrect documentation to be a function topic (not dataset).
man/filterSeuratObjectByMetadata.Rd Updates title/description and adds usage example.
man/filterQC.Rd Fixes parameter typos, renames args, and adds example.
man/dualLabeling.Rd Updates defaults/parameter names and adds example.
man/dotPlotMet.Rd Adds examples section.
man/degGeneExpressionMarkers.Rd Updates title/description and adds example.
man/compareCellPopulations.Rd Adds documentation for new compareCellPopulations function.
man/combineNormalize.Rd Adds examples section.
man/colorByGene.Rd Adds examples section.
man/annotateCellTypes.Rd Updates title/description and adds example.
man/aggregateCounts.Rd Updates signature (defaults/interactive) and adds example.
inst/shiny/ModuleScoreApp/app_011726.R Refactors Shiny app variable naming to dotted style and matches helper signatures.
docs/sitemap.xml Updates sitemap entries for new/renamed pages.
docs/reference/tSNE3D.html Regenerates reference page with value/examples sections.
docs/reference/processRawData.html Regenerates reference page with examples section.
docs/reference/plotMetadata.html Regenerates reference page with examples section.
docs/reference/object.html Regenerates reference page content (harmonyBatchCorrect example section).
docs/reference/nameClusters.html Regenerates reference page reflecting new argument semantics/examples.
docs/reference/modscore-imports-011726.html Adds generated pkgdown page for internal helper topic.
docs/reference/launch_module_score_app.html Adds generated pkgdown page for app launcher.
docs/reference/index.html Updates function index listing/labels and adds new topics.
docs/reference/heatmapSC.html Regenerates reference page reflecting use.assay and examples.
docs/reference/filterSeuratObjectByMetadata.html Regenerates reference page with new title/description/examples.
docs/reference/dotPlotMet.html Regenerates reference page with examples.
docs/reference/degGeneExpressionMarkers.html Regenerates reference page with new title/description/examples.
docs/reference/annotateCellTypes.html Regenerates reference page with new title/description/examples.
docs/reference/compute_modscore_data.html Adds redirect shim page to modscore-imports topic.
docs/reference/combineNormalize.html Regenerates reference page with examples.
docs/reference/colorByMarkerTable.html Regenerates reference page and reflects new args/docs sections.
docs/reference/colorByGene.html Regenerates reference page with examples.
docs/pkgdown.yml Updates pkgdown build timestamp.
docs/index.html Updates site landing page content and license links.
docs/deps/Roboto_Mono-0.4.9/font.css Adds font dependency asset for pkgdown output.
docs/decision_log.html Adds generated decision log page.
docs/authors.html Updates citation year.
docs/articles/index.html Fixes README article title in articles index.
docs/articles/SCWorkflow-SubsetReclust.html Regenerates article reflecting quote/style changes.
docs/articles/README.html Regenerates README article page with correct title.
docs/LICENSE.html Adds generated full license page.
docs/LICENSE-text.html Updates license-text rendering.
docs/CHANGELOG.html Regenerates changelog HTML including v1.0.3 in-development section.
README.md Adds “Key Functions” overview section.
R/Violin_Plots_by_Metadata.R Renames violinPlot_modviolinPlot, updates args, and changes return to list(plots=...).
R/Process_Raw_Data.R Adds examples block.
R/Plot_Metadata.R Adds examples and refactors parsing/selection logic for samples/metadata arguments.
R/Name_Clusters_by_Enriched_Cell_Type.R Updates nameClusters API to use a mapping table and adds examples.
R/ModuleScoreHelpers_011726.R Refactors helper signatures to dotted style and aligns import directives.
R/ModuleScoreHelpers.R Refactors helper signatures/imports and adds examples.
R/Heatmap.R Renames args (use.assay) and adds examples.
R/Harmony.R Refactors signature/return to include plots; adds patchwork import and modifies logging/assay creation.
R/Filter_Seurat_Object_by_Metadata.R Updates title/description and adds example.
R/Dual_Labeling.R Updates docs and output structure (data/plots) and expands reduction options.
R/Dotplot_by_Metadata.R Adds examples and minor doc formatting.
R/DEG_Gene_Expression_Markers.R Updates docs and adds examples.
R/Combine_and_Normalize.R Adds examples section.
R/Color_by_Genes_Automatic.R Refactors return into plots list and adjusts formatting.
R/Color_by_Gene.R Adds examples and refactors sample parsing.
R/Annotate_Cell_Types.R Updates docs and adds examples.
R/AggregateCounts.R Adds interactive option and changes return to list(data=..., plots=...).
R/3D_tSNE.R Adds return docs and examples.
NAMESPACE Updates exports/imports to reflect renamed functions and new helpers.
Dockerfile Pins R to 4.1.3, expands installed package set, and adds dependency verification step.
CHANGELOG.md Adds v1.0.3 (in development) entry for compareCellPopulations feature.
.gitignore Updates ignored paths (doc/Meta) and removes some previous patterns.
.github/workflows/user-projects.yml Adds workflow to auto-add assigned issues/PRs to a project.
.github/workflows/test-coverage.yaml Adds coverage workflow using covr + codecov.
.github/workflows/pkgdown.yaml Updates pkgdown build workflow (container-based, R 4.1.3).
.github/workflows/docker.yml Updates Docker build arg R_VERSION to 4.1.3.
.github/workflows/R-CMD-check.yaml Adds R CMD check + lint + coverage workflows.
.github/package-versions.txt Adds package pin file for CI installs.
.github/PULL_REQUEST_TEMPLATE.md Adds PR template with checklist.
.github/ISSUE_TEMPLATE/feature_request.yml Adds feature request issue template.
.github/ISSUE_TEMPLATE/config.yml Adds issue template config with discussions link.
.github/ISSUE_TEMPLATE/bug_report.yml Adds bug report issue template.
.Rhistory Updates recorded parameter names in history file.
.Rbuildignore Ignores doc/ and Meta/ directories for package builds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread R/Harmony.R
Comment on lines 233 to 238

# Insert back-calculated data into seurat
# Insert back-calculated data into seurat
print( "Insert back-calculated data into seurat")
object[["Harmony"]] <- CreateAssayObject(data = harm.lvl.backcalc.lognorm)
#object[["Harmony"]] <- CreateAssayObject(data = Matrix::Matrix(t(harm.lvl.backcalc.lognorm), sparse = TRUE))
object@assays$Harmony@scale.data <- t(harm.lvl.backcalc.scaled)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If return.lognorm is FALSE, harm.lvl.backcalc.lognorm is never defined, but it is still used to create the Harmony assay. Gate the assay creation on return.lognorm or define an alternative matrix for the FALSE path.

Copilot uses AI. Check for mistakes.
Comment thread R/ModuleScoreHelpers.R
Comment on lines 1 to 6
#' @title Helpers for ModuleScore Shiny app
#' @description Precompute module scores per celltype and build plots from cached data.
#' @name modscore-imports
#' @keywords internal
#' @importFrom dplyr mutate group_by summarise arrange select
#' @importFrom ggplot2 ggplot aes theme_bw theme element_blank
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These helpers are marked @keywords internal but are also exported (@export and NAMESPACE). Align intent: keep them internal (remove exports) or treat them as public API (remove internal keyword and document stability).

Copilot uses AI. Check for mistakes.
Comment thread R/AggregateCounts.R
Comment on lines +100 to 102
return(list(data=pseudobulk,
plots=p))
} No newline at end of file
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function now returns list(data = ..., plots = ...), but the roxygen @return still documents a data.frame. Update the return documentation (and any generated Rd) to reflect the list structure.

Copilot uses AI. Check for mistakes.
Comment on lines +178 to 183
if (jitter.points) {
g <- g + geom_jitter(size = jitter.dot.size, shape = 1, position = position_dodge(width = 0.9), alpha = 0.5)
}

return(g)
return(list("plots"=g))
}
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function now returns list(plots = g) but the documentation/generated Rd still describe returning a ggplot object. Update the @return docs accordingly (or return the ggplot directly for backward compatibility).

Copilot uses AI. Check for mistakes.
Comment on lines +16 to 19
facet.by = ""
set.seed(81)
genes = sample(rownames(Seurat::GetAssayData(object, assay = assay, layer = layer)), 5,
replace = FALSE)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper calls Seurat::GetAssayData(..., layer = layer), which will fail on Seurat 4.x where the argument is slot. Tests using this helper will break under the repo's pinned Seurat 4.1.1; make the call version-conditional (slot vs layer) to support both Seurat 4 and 5.

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +55
violinPlot <- function (object,
assay,
layer,
genes,
group,
facet_by = "",
filter_outliers = F,
outlier_low = 0.05,
outlier_high = 0.95,
jitter_points,
jitter_dot_size)
facet.by = "",
filter.outliers = F,
outlier.low = 0.05,
outlier.high = 0.95,
jitter.points,
jitter.dot.size)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function’s API now uses layer (and later calls GetAssayData(..., layer=...)), but Seurat 4.x uses slot instead of layer. Since SCWorkflow supports Seurat >= 4.1.1, consider accepting both (slot as an alias) and dispatching to slot vs layer based on Seurat version to avoid runtime errors.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants