-
Notifications
You must be signed in to change notification settings - Fork 597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
validatevariants -gvcf fix #3530
Conversation
…s followed by a record that the previous one fully encompasses. The interval math was off in this case
Codecov Report
@@ Coverage Diff @@
## master #3530 +/- ##
==============================================
+ Coverage 80.079% 80.109% +0.03%
- Complexity 17760 17791 +31
==============================================
Files 1188 1188
Lines 64410 64543 +133
Branches 10004 10022 +18
==============================================
+ Hits 51579 51705 +126
Misses 8845 8845
- Partials 3986 3993 +7
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one possible optimization, otherwise 👍
@@ -205,10 +205,13 @@ public void apply(final VariantContext vc, final ReadsContext readsContext, fina | |||
// if the current record is adjacent to the previous record and "overlap" them if they are so our set is as | |||
// small as possible while still containing the same bases. | |||
final int start = (previousInterval != null && previousInterval.overlapsWithMargin(refInterval, 1)) ? | |||
previousInterval.getEnd() : refInterval.getStart(); | |||
genomeLocSortedSet.add(genomeLocSortedSet.getGenomeLocParser().createGenomeLoc(refInterval.getContig(), start, vc.getEnd()), true); | |||
Math.min(previousInterval.getStart(), refInterval.getStart()) : refInterval.getStart(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this:
Math.min(previousInterval.getStart(), refInterval.getStart())
not always equal previousInterval.getStart()
given the order of iteration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@lbergelson fix for an incorrect assumption I had made previously, if you or someone else could review thanks! |
fix issue with validatevariants when validatign a gvcf and a record is followed by a record that is fully encompassed by the first one. The interval math was off in this case