Development - #71
Open
alex-sandercock wants to merge 45 commits into
Open
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ement for documentation on check_ped
…and to plot results.
… or file paths, updated documentation and test files
…ring genotype matrices for comparisons. updated test file to accomodate nw arguments
Replaced separate with extract to parse SNP ID into CHROM and POS. Updated POS formatting to handle leading zeros.
Update updog2vcf
Add parentage functions
…atches
When add_others = TRUE, compare() computed other_ref_base/other_alt_base with
substring() before checking that any mismatch positions remained. If an off-target
("Other") allele aligns to the reference with its only mismatch at the target SNP
position, that position is removed and pos_ref_idx/pos_alt_idx become integer(0),
so substring() errors with "invalid substring arguments" and the whole run aborts.
Move the two substring() calls inside the existing 'if (length(pos_ref_idx) > 0)'
guard, mirroring how the off-target Match alleles are already handled. Behavior is
unchanged when mismatch positions remain; existing madc2vcf_all tests still pass.
…tring Fix madc2vcf_all() "invalid substring arguments" error on off-target alleles
Filter vcf bug fixes
Contributor
There was a problem hiding this comment.
🔵 Human review recommended
It touches multiple core data-processing paths (MADC→VCF conversion and VCF filtering) while also removing public APIs, so a final human review is needed to confirm behavioral compatibility and release readiness.
Pull request overview
This PR prepares the BIGr 0.9.0 release by tightening MADC/VCF handling (especially around IUPAC codes and POS parsing), fixing long-standing filterVCF() INFO parsing issues, and cleaning out legacy pedigree/parentage/breed-composition functionality that has been moved elsewhere.
Changes:
- Expanded
check_madc_sanity()IUPAC reporting (distinguish differing vs identical-position IUPAC in REF/ALT, and IUPAC in Match/Other alleles) and alignedmadc2vcf_*error/warning behavior accordingly. - Added new “Other allele” filtering controls to
madc2vcf_all()and hardened VCF POS generation/sorting to enforce positive integer positions (including scientific-notation correction). - Reworked
filterVCF()INFO parsing (scientific notation, missing/unreadable values) and updated tests/docs; removed legacy pedigree/parentage and breed composition functions + docs + tests.
File summaries
| File | Description |
|---|---|
| tests/testthat/test-validate_pedigree.R | Removed tests for validate_pedigree() (function removed from package). |
| tests/testthat/test-madc2vcf_multi.R | Updated expected IUPAC-related error message for madc2vcf_multi(). |
| tests/testthat/test-madc2vcf_all.R | Updated tests to expect warnings and revised error messages for marker ID matching / IUPAC handling. |
| tests/testthat/test-find_parentage.R | Removed tests for find_parentage() (function removed from package). |
| tests/testthat/test-filterVCF.R | Added/updated regression tests for INFO scientific-notation parsing and unreadable-value handling. |
| tests/testthat/test-check_ped.R | Removed tests for check_ped() (function removed from package). |
| tests/testthat/test-check_madc_sanity.R | Updated expected check set/ordering and expectations for new IUPAC checks. |
| tests/testthat/test-breedtools_poly.R | Removed tests for legacy breed composition functions (moved to BIGpopA). |
| tests/testthat/corrected_pedigree.txt | Removed tracked output artifact from prior pedigree tests. |
| README.md | Updated citation block. |
| R/validate_pedigree.R | Removed validate_pedigree() implementation. |
| R/updog2vcf.R | Improved CHROM/POS extraction and POS normalization (strip leading zeros safely). |
| R/madc2vcf_targets.R | Added POS validation/correction and robust sorting; updated imports. |
| R/madc2vcf_multi.R | Updated IUPAC error messaging and marker ID column guidance. |
| R/madc2vcf_all.R | Added new “Other” allele filters and more robust IUPAC/POS handling throughout conversion. |
| R/imputation_concordance.R | Refreshed documentation and imports; clarified examples and plotting imports. |
| R/find_parentage.R | Removed find_parentage() implementation. |
| R/filterVCF.R | Fixed INFO value parsing (scientific notation, exact field matching), hardened filtering against NA indices, improved docs/validation. |
| R/check_ped.R | Removed check_ped() implementation. |
| R/check_madc_sanity.R | Added new checks and outputs for IUPAC positioning/equality and Match/Other alleles. |
| R/breedtools_functions.R | Removed legacy breed composition functions (moved to BIGpopA). |
| NEWS.md | Added BIGr 0.9.0 changelog and expanded notes for 0.8.1 fixes. |
| NAMESPACE | Removed exports/imports for deleted APIs; added ggplot2 imports and cleaned up imports. |
| man/validate_pedigree.Rd | Removed Rd for deleted function. |
| man/solve_composition_poly.Rd | Removed Rd for deleted function. |
| man/madc2vcf_all.Rd | Documented new others_min_dist / others_max_close_snps arguments and updated markers_info ID guidance. |
| man/imputation_concordance.Rd | Synced Rd with updated roxygen/docs. |
| man/find_parentage.Rd | Removed Rd for deleted function. |
| man/filterVCF.Rd | Updated documentation to match new filtering semantics and return behavior. |
| man/check_ped.Rd | Removed Rd for deleted function. |
| man/check_madc_sanity.Rd | Updated documentation for new checks/return fields. |
| man/allele_freq_poly.Rd | Removed Rd for deleted function. |
| inst/check_ped_test.txt | Updated header/fixture contents for pedigree-related test data. |
| dev/dev_history.R | Updated release/dev helper script. |
| DESCRIPTION | Bumped version to 0.9.0; adjusted dependencies/imports/suggests and metadata formatting. |
| .Rbuildignore | Ignored additional local tooling directories. |
| .gitignore | Ignored .positai directory. |
Review details
Files not reviewed (4)
- man/check_madc_sanity.Rd: Generated file
- man/filterVCF.Rd: Generated file
- man/imputation_concordance.Rd: Generated file
- man/madc2vcf_all.Rd: Generated file
Suppressed comments (1)
R/madc2vcf_all.R:742
- Spelling in comment: "polymorphis"/"discarted" should be "polymorphism"/"discarded".
if(length(other_alt_base) >0){ # If the N or IUPAC code is the only polymorphis found, the Match tag will be discarted
- Files reviewed: 32/37 changed files
- Comments generated: 5
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| If you use BIGr in your research, please cite as: | ||
|
|
||
| Sandercock, Alexander M., Cristiane H. Taniguti, Josue Chinchilla-Vargas, Dongyan Zhao, Shufen Chen, Meng Lin, Manoj Sapkota, and Breeding Insight Team. 2025. “Breeding Insight Genomics Functions for Polypoid and Diploid Species.” https://github.com/Breeding-Insight/BIGr. | ||
| Sandercock A.M., Peel M.D., Taniguti C.H., Chinchilla-Vargas J., Chen S., Sapkota M., Lin M., Zhao D., Ackerman A.J., Basnet B.R., Beil C.T., Sheehan M.J. (2025). BIGapp: A User-Friendly Genomic Tool Kit Identified Quantitative Trait Loci for Creeping Rootedness in Alfalfa (Medicago sativa L.)., The Plant Genome. doi:https://doi.org/10.1002/tpg2.70067 |
| * `others_max_close_snps` (default 3 SNPs) | ||
|
|
||
| * By default, `Other` tags will be discarded if they have more than 3 SNPs with less than 5bp distance between them | ||
| * Adapt `madc2vcf_all` code to let pass identical IUPAC code in identical position in REF/ALT but ignore polymorphims in Match or Other alleles at the same position |
| } | ||
| other_ref_base <- substring(ref_seq, pos_ref_idx, pos_ref_idx) | ||
| other_alt_base <- substring(others_seq[j,]$AlleleSequence, pos_alt_idx, pos_alt_idx) | ||
| # Cases found where the AltMatch is another alternative for the target SNP - they are discarted |
| # Update dependencies in DESCRIPTION | ||
| # install.packages('attachment', repos = 'https://thinkr-open.r-universe.dev') | ||
| #attachment::att_amend_desc() | ||
| attachment::att_amend_desc() |
| alt_base_match <- alt_base_match[-rm_pos] | ||
| } | ||
|
|
||
| if(length(alt_base_match) >0){ # If the N is the only polymorphis found, the Match tag will be discarted |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 82.02% 81.34% -0.69%
==========================================
Files 23 19 -4
Lines 2876 2385 -491
==========================================
- Hits 2359 1940 -419
+ Misses 517 445 -72 ☔ 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 introduces BIGr version 0.9.0, which includes important feature enhancements, bug fixes, and codebase cleanup. The main updates focus on improving the handling and validation of IUPAC codes in allele sequences, adding new arguments and logic to the
madc2vcf_allfunction, and removing legacy breed composition functions that have been moved to a separate package.Key highlights:
madc2vcf_all.filterVCF()and VCF export logic.MADC and VCF Handling Improvements
check_madc_sanityto distinguish between presence of IUPAC codes, their positions, and their presence in Ref/Alt/Other alleles, with new logical outputs:IUPACcodes,IUPACcodes_IdenticalRefAlt, andIUPACcodes_MatchAlleles. The documentation now describes twelve validation checks instead of nine. [1] [2] [3]madc2vcf_allnow supports two new arguments:others_min_dist(default 5bp) andothers_max_close_snps(default 3 SNPs), allowing the discarding of "Other" tags with too many close SNPs by default.madc2vcf_all,madc2vcf_targets, andmadc2vcf_multito handle IUPAC codes more robustly, only throwing errors when different IUPACs are present in REF/ALT.madc2vcf_all()to ensure exported positions are always integers and negative positions are removed. Also fixed an "invalid substring arguments" error when handling off-target alleles.VCF Filtering Bug Fixes
filterVCF()to properly parse INFO values in scientific notation, avoid writing corrupt all-NA variants, and ensure filters are correctly applied regardless of INFO field order or presence. Documentation was also corrected and expanded.Package Cleanup and Dependency Updates
allele_freq_poly,solve_composition_poly, etc.) from the codebase and NAMESPACE, as these are now part of the separate BIGpopA package. [1] [2] [3]DESCRIPTION: bumped version to 0.9.0, revised dependencies (addedggplot2, removedjanitor,quadprog,data.table), and improved formatting.Documentation
NEWS.md, summarizing all major changes and fixes.