Skip to content

Development - #58

Merged
josuechinchilla merged 21 commits into
add_parentage_functionsfrom
development
Apr 22, 2026
Merged

Development#58
josuechinchilla merged 21 commits into
add_parentage_functionsfrom
development

Conversation

@josuechinchilla

Copy link
Copy Markdown
Collaborator

Added original parentage functions and updated those to fit Meng's feedback

@josuechinchilla
josuechinchilla merged commit f590dad into add_parentage_functionsApr 22, 2026
4 checks passed

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 improves the MADC→VCF conversion utilities and related sanity checks, adds regression tests for edge cases (sample name preservation, markers_info ID matching, AD formatting), and updates package metadata/docs for optional dependencies and internal helpers.

Changes:

  • Add tests covering sample-name preservation, missing-column error surfacing, AD formatting, and markers_info ID remapping behavior.
  • Make sanity-check reporting robust to NA checks and add Marker_ID support via a shared ID-column picker helper.
  • Update docs and package metadata: make ggplot2/polyRAD optional at runtime, and adjust exported/internal utilities.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
tests/testthat/test-madc2vcf_targets.RAdds regression tests for sample names and missing-column errors; updates expected error text.
tests/testthat/test-madc2vcf_all.RAdds regression tests for AD formatting, BI_markerID matching behavior, and missing-column errors; loosens one error regex.
tests/testthat/test-check_madc_sanity.RAdds skip-if-offline and several new tests around botloci remapping/padding and ID-column selection.
man/vmsg.RdRemoves generated documentation for vmsg() (now treated as internal).
man/madc2vcf_targets.RdUpdates parameter docs to reflect Marker_ID support and botloci/markers_info requirements.
man/madc2vcf_multi.RdUpdates description to reflect multiallelic/microhaplotype pipeline wording.
man/imputation_concordance.RdReplaces \dontrun{} example with runnable example data and call.
man/get_countsMADC.RdUpdates \seealso{} to remove link to internal get_counts().
man/get_counts.RdRemoves generated documentation for internal get_counts().
man/filterVCF.RdRemoves quality.rates argument docs and updates examples.
man/check_madc_sanity.RdDocuments the new OtherAlleles check.
R/utils.RConverts vmsg() roxygen to @noRd (internal) and minor formatting cleanup.
R/madc2vcf_targets.RPreserves original column names (check.names=FALSE), handles NA checks safely, and uses shared ID-column picking (CloneID/Marker_ID/BI_markerID).
R/madc2vcf_multi.RMakes sanity-check messaging robust to NA and adds a runtime check for optional polyRAD.
R/madc2vcf_all.RMakes sanity-check messaging robust to NA, improves markers_info ID matching logic, and fixes AD formatting for biallelic targets.
R/imputation_concordance.RRemoves ggplot2 import reliance; uses ggplot2:: and checks requireNamespace() when plot=TRUE.
R/get_countsMADC.REnforces mutual exclusivity of madc_file vs madc_object; marks internal get_counts() as @noRd.
R/filterVCF.RRemoves quality.rates functionality and shifts messaging from cat() to message()/warning().
R/check_madc_sanity.RImproves ChromPos validation, adds pick_markers_info_id_col(), and enhances check_botloci() remapping/padding behavior.
NAMESPACEStops exporting vmsg() and removes ggplot2 import.
DESCRIPTIONMoves ggplot2 to Suggests (optional dependency).

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

Pos = c(1, 2)
)

res <- check_botloci(botloci, report, ChromPos = FALSE, mi_df = mi_df, verbose = FALSE)

CopilotAIApr 22, 2026

Copy link

Choose a reason for hiding this comment

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

These tests call check_botloci() directly, but check_botloci is not exported in NAMESPACE. testthat::test_check() loads the installed package via library(BIGr), so this will fail with “could not find function” in CI/CRAN. Call it via BIGr:::check_botloci() (or export it if it’s intended to be public).

Copilot uses AI. Check for mistakes.
Pos = 123
)

res <- check_botloci(botloci, report, ChromPos = TRUE, mi_df = mi_df, verbose = FALSE)

CopilotAIApr 22, 2026

Copy link

Choose a reason for hiding this comment

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

These tests call check_botloci() directly, but check_botloci is not exported in NAMESPACE. testthat::test_check() loads the installed package via library(BIGr), so this will fail with “could not find function” in CI/CRAN. Call it via BIGr:::check_botloci() (or export it if it’s intended to be public).

Copilot uses AI. Check for mistakes.
)
query_ids <- c("m1", "m1", "m1", "m2")

expect_equal(pick_markers_info_id_col(mi_df, query_ids), "CloneID")

CopilotAIApr 22, 2026

Copy link

Choose a reason for hiding this comment

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

This test calls pick_markers_info_id_col() directly, but it is an internal helper (@noRd) and not exported. Under testthat::test_check() (installed package), this will error. Use BIGr:::pick_markers_info_id_col() (or export it if it’s intended to be public/tested).

Suggested change
expect_equal(pick_markers_info_id_col(mi_df, query_ids), "CloneID")
expect_equal(BIGr:::pick_markers_info_id_col(mi_df, query_ids), "CloneID")

Copilot uses AI. Check for mistakes.
)

expect_error(
check_botloci(botloci, report, ChromPos = TRUE, verbose = FALSE),

CopilotAIApr 22, 2026

Copy link

Choose a reason for hiding this comment

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

These tests call check_botloci() directly, but check_botloci is not exported in NAMESPACE. testthat::test_check() loads the installed package via library(BIGr), so this will fail with “could not find function” in CI/CRAN. Call it via BIGr:::check_botloci() (or export it if it’s intended to be public).

Copilot uses AI. Check for mistakes.
Pos = 1
)

res <- check_botloci(botloci, report, ChromPos = TRUE, mi_df = mi_df, verbose = FALSE)

CopilotAIApr 22, 2026

Copy link

Choose a reason for hiding this comment

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

These tests call check_botloci() directly, but check_botloci is not exported in NAMESPACE. testthat::test_check() loads the installed package via library(BIGr), so this will fail with “could not find function” in CI/CRAN. Call it via BIGr:::check_botloci() (or export it if it’s intended to be public).

Copilot uses AI. Check for mistakes.
Comment threadR/filterVCF.R
Comment on lines 44 to +55
filterVCF <- function(vcf.file,
quality.rates = FALSE,
filter.OD = NULL,
filter.BIAS.min = NULL,
filter.BIAS.max = NULL,
filter.DP = NULL,
filter.MPP = NULL,
filter.PMC = NULL,
filter.MAF = NULL,
filter.SAMPLE.miss = NULL,
filter.SNP.miss = NULL,
ploidy,
output.file = NULL) {
filter.OD = NULL,
filter.BIAS.min = NULL,
filter.BIAS.max = NULL,
filter.DP = NULL,
filter.MPP = NULL,
filter.PMC = NULL,
filter.MAF = NULL,
filter.SAMPLE.miss = NULL,
filter.SNP.miss = NULL,
ploidy,
output.file = NULL) {

CopilotAIApr 22, 2026

Copy link

Choose a reason for hiding this comment

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

filterVCF() is exported and this change removes the quality.rates argument entirely. That’s a breaking API change for any callers passing quality.rates=.... Consider keeping the parameter (deprecated/no-op with a warning) or accepting ... and warning on quality.rates to preserve backward compatibility for at least one release.

Copilot uses AI. Check for mistakes.
Comment threadNAMESPACE
Comment on lines 20 to 24
export(madc2vcf_multi)
export(madc2vcf_targets)
export(merge_MADCs)
export(solve_composition_poly)
export(thinSNP)

CopilotAIApr 22, 2026

Copy link

Choose a reason for hiding this comment

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

NAMESPACE no longer exports vmsg. If vmsg() was previously part of the public API, this is a breaking change for downstream users. If the intent is to make it internal, consider a deprecation period (keep exporting but mark deprecated) and document the change in NEWS / bump version accordingly.

Copilot uses AI. Check for mistakes.
Sign up for freeto 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.

4 participants

@josuechinchilla@alex-sandercock@Cristianetaniguti