Conversation
* 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>
There was a problem hiding this comment.
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.,plotsvsplot,*.by/*.varconventions). - 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 (FilteringTables → data) 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 (df → data). |
| tests/testthat/test-Color_by_Gene.R | Updates expected output keys and plot indexing from plot → plots. |
| 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_columns→use.columns, ms_threshold→ms.threshold) and adds group.var. |
| man/launch_module_score_app.Rd | Adds documentation for launching the ModuleScore Shiny app. |
| man/heatmapSC.Rd | Renames use_assay→use.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_mod → violinPlot, 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.
|
|
||
| # 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) |
There was a problem hiding this comment.
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.
| #' @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 |
There was a problem hiding this comment.
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).
| return(list(data=pseudobulk, | ||
| plots=p)) | ||
| } No newline at end of file |
There was a problem hiding this comment.
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.
| 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)) | ||
| } |
There was a problem hiding this comment.
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).
| facet.by = "" | ||
| set.seed(81) | ||
| genes = sample(rownames(Seurat::GetAssayData(object, assay = assay, layer = layer)), 5, | ||
| replace = FALSE) |
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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.
No description provided.