Filter vcf bug fixes - #68
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves filterVCF()’s correctness and transparency by fixing INFO-field parsing (notably scientific notation), making INFO-based filters consistently applied across all records, and ensuring variants with unreadable/missing filter values are removed (with explicit warnings) rather than producing corrupt all-NA rows.
Changes:
- Reworked INFO field extraction to correctly parse full numeric representations (including scientific notation) and to match full INFO entries (avoiding substring collisions).
- Added explicit removal + warnings for variants with missing/unreadable values needed by requested filters (including
MAF). - Updated documentation (Roxygen + Rd), NEWS entry, and expanded test coverage to lock in the new behaviors.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
R/filterVCF.R |
Fix INFO parsing + apply INFO filters consistently; remove variants with unusable filter values and warn. |
tests/testthat/test-filterVCF.R |
Adds/updates tests covering scientific notation parsing and removal of unreadable/missing filter values. |
man/filterVCF.Rd |
Regenerated Rd documenting filter semantics/order and return behavior. |
NEWS.md |
Changelog entries describing the filtering/parsing fixes and doc updates. |
Files not reviewed (1)
- man/filterVCF.Rd: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- man/filterVCF.Rd: Generated file
Suppressed comments (1)
R/filterVCF.R:211
select_variants()only treatsNAvalues invaluesas unusable. If the filter comparison itself producesNA(e.g., because a user suppliesfilter.OD = NAor another threshold coerces toNA_real_),keepwill containNAs andvcf[keep, ]can again insert corrupt all-NAvariants—the behavior this PR is trying to eliminate. Ensurekeepnever containsNAbefore subsetting (and ideally warn when comparisons evaluate toNA).
select_variants <- function(keep, values, label, hint) {
unusable <- is.na(values)
if (length(values) > 0 && all(unusable)) {
# Losing every variant is usually a problem with the input rather than a
# genuine result, so this case says what to look at instead of only
# reporting the count.
warning("No readable ", label, " values were found, so all ", length(values),
" variants were removed. ", hint, call. = FALSE)
} else if (any(unusable)) {
warning(sum(unusable), " of ", length(values), " variants had a missing or ",
"unreadable ", label, " value and were removed.", call. = FALSE)
}
return(keep & !unusable)
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## SAT-151-allow-iupac-codes-on-match-alleles #68 +/- ##
==============================================================================
+ Coverage 80.92% 81.34% +0.41%
==============================================================================
Files 19 19
Lines 2349 2385 +36
==============================================================================
+ Hits 1901 1940 +39
+ Misses 448 445 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
13628c3 to
cbcfb19
Compare
This pull request makes several important improvements and bug fixes to the
filterVCF()function and its documentation, focusing on more robust handling of VCF INFO field parsing, clearer and more accurate documentation, and improved filtering logic. The changes ensure that filtering is consistent, transparent, and less error-prone, especially when dealing with scientific notation and missing values.Bug Fixes and Filtering Logic Improvements:
PMC=4.78e-07) by usingas.numeric()on the entire field, ensuring correct filtering of variants and preventing the loss of high-quality SNPs. [1] [2]OD,BIAS,PMC, or MAF) are now explicitly removed and the number removed is reported in a warning, preventing the creation of corrupt all-NAvariants and ensuring users are aware of data loss. [1] [2] [3]Documentation and Usability Improvements:
filterVCF()(in both Roxygen and Rd formats) has been corrected and expanded to accurately describe each filter's behavior, clarify the filtering order, and specify what is returned by the function. [1] [2] [3] [4]These changes make the filtering process more reliable, transparent, and user-friendly, reducing the risk of silent data loss or unexpected results.