New features and updated for MADC handling - #72
Open
alex-sandercock wants to merge 6 commits into
Open
Conversation
alex-sandercock
added
enhancement
Contributor
There was a problem hiding this comment.
Pull request overview
This PR releases BIGr v0.10.0 with expanded support for fixed-allele-ID MADC workflows: it introduces new MADC summarization/plotting APIs and revises filterMADC() to enforce HapApp-processed inputs while preserving Ref/Alt pairing during filtering.
Changes:
- Added
madc_summary()andmadc_plot()(plus shared internal helpers) for fixed-allele-ID MADC reporting and visualization. - Reworked
filterMADC()to validate fixed AlleleIDs viacheck_madc_sanity(), replace per-mhap depth filters with per-locus depth windows, and protect target Ref/Alt rows from pruning. - Updated package metadata/docs/tests (version bump, NEWS, NAMESPACE exports, new test coverage, wordlist updates).
Reviewed changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/test-madc_summary.R | Adds unit tests covering madc_summary() outputs, metadata grouping, CSV writing, and raw-file rejection. |
| tests/testthat/test-madc_plot.R | Adds unit tests for madc_plot() plot types, multi-panel behavior, circos gating, metadata options, and raw-file rejection. |
| tests/testthat/test-filterMADC.R | Updates tests to new filterMADC() arguments/invariants and adds fixtures for ` |
| R/utils.R | Registers additional ggplot2 NSE symbols used by new plotting code. |
| R/madc_utils.R | Introduces shared internal helpers for reading/validating MADC input, computing metrics, grouping, arranging grobs, and saving plots. |
| R/madc_summary.R | Implements new exported madc_summary() function and its table outputs. |
| R/madc_plot.R | Implements new exported madc_plot() function (PCA/marker/heatmap/missing/circos). |
| R/filterMADC.R | Enforces fixed-allele-ID input, adds per-locus depth window filtering, and revises pruning logic to preserve Ref/Alt. |
| NEWS.md | Adds 0.10.0 release notes describing new features and breaking changes. |
| NAMESPACE | Exports madc_plot and madc_summary. |
| man/madc_summary.Rd | Generated documentation for madc_summary(). |
| man/madc_plot.Rd | Generated documentation for madc_plot(). |
| man/filterMADC.Rd | Updates generated documentation for revised filterMADC() interface/behavior. |
| inst/WORDLIST | Adds new domain terms for spell-checking (e.g., HapApp, mHaps, circos). |
| DESCRIPTION | Bumps package version and updates Imports/Suggests for new plotting functionality. |
Files not reviewed (3)
- man/filterMADC.Rd: Generated file
- man/madc_plot.Rd: Generated file
- man/madc_summary.Rd: Generated file
Suppressed comments (1)
R/filterMADC.R:32
- The documented return value (
data.frame or saved csv file) is ambiguous and doesn’t match the current behavior: whenoutput.fileis non-NULL the function writes the CSV and returnsNULL(it only returns a data.frame whenoutput.fileis NULL). Updating the@returntext will prevent user confusion.
#'@param output.file Path to save the filtered data (if NULL, data will not be saved)
#'
#'@return data.frame or saved csv file
#'
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+159
to
+171
| .madc_group <- function(metadata, group.col, samples) { | ||
| if (is.null(metadata)) return(NULL) | ||
| if (!is.data.frame(metadata)) stop("`metadata` must be a data.frame.") | ||
| if (is.null(group.col) || !group.col %in% names(metadata)) | ||
| stop("`group.col` must be a column name in `metadata`.") | ||
| id_candidates <- intersect(c("sample", "Sample", "ID", "SampleID", "sample_id", | ||
| "Sample_ID"), names(metadata)) | ||
| id_col <- if (length(id_candidates)) id_candidates[1] else names(metadata)[1] | ||
| grp <- as.character(metadata[[group.col]][match(samples, metadata[[id_col]])]) | ||
| if (all(is.na(grp))) | ||
| warning("No `metadata` sample IDs matched the MADC sample columns; grouping skipped.") | ||
| grp | ||
| } |
Comment on lines
+211
to
+215
| pc <- stats::prcomp(mat, center = TRUE, scale. = FALSE) | ||
| ve <- pc$sdev^2 / sum(pc$sdev^2) | ||
| df <- data.frame(sample = rownames(mat), | ||
| PCx = pc$x[, pc.x], PCy = pc$x[, pc.y], | ||
| stringsAsFactors = FALSE) |
Comment on lines
+46
to
+48
| #' @param facet.chrom Logical; facet the heatmap by chromosome. Default `FALSE`. | ||
| #' When `metadata`/`group.col` are supplied, the heatmap also sorts samples by | ||
| #' category and labels each category (a facet column per group). |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #72 +/- ##
===============================================
+ Coverage 81.34% 83.66% +2.32%
===============================================
Files 19 22 +3
Lines 2385 3042 +657
===============================================
+ Hits 1940 2545 +605
- Misses 445 497 +52 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the BIGr package to version 0.10.0, introducing two new functions for MADC file analysis and making significant improvements to the
filterMADCworkflow. The changes focus on supporting only fixed allele ID MADC files, refining filtering logic for better data integrity, and enhancing plotting and summary capabilities.Major new features:
madc_summary()for generating detailed summary tables of fixed allele ID MADC files, including per-marker and per-sample statistics, missingness sweeps, and optional aggregation by metadata category. [1] [2]madc_plot()for versatile visualization of MADC data, supporting PCA, genome plots, heatmaps, boxplots, and publication-ready circos plots, with broad customization and export options. [1] [2]Core updates to MADC filtering:
filterMADCnow exclusively accepts fixed allele ID MADC files (processed through HapApp), with input validation viacheck_madc_sanity. Raw DArT MADC files or those lacking fixed AlleleIDs are rejected with a clear error. [1] [2]n.summary.columnsargument (breaking change) and switches from per-mhap read-depth filtering to a per-locus depth window (min.locus.depth/max.locus.depth), ensuring loci are only removed as a whole and always retain their Ref/Alt pair. [1] [2]max.mhaps.per.loci,target.only, andmin.ind.with.readshas been revised to always preserve the target Ref/Alt alleles, only pruning non-target (|RefMatch,|AltMatch,|Other) mhaps. [1] [2]Dependency and metadata updates:
DESCRIPTION.grDevices,graphics,grid,tools) and suggests (circlize) to support new plotting features. [1] [2]Documentation:
filterMADC, reflecting the new input requirements and filtering behavior. [1] [2]NEWS.mddescribing new features and breaking changes.