-
Notifications
You must be signed in to change notification settings - Fork 594
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
Fix reported bug where no-call genotypes with no reads get genotype #5667
Conversation
posterior probabilities and calls
Codecov Report
@@ Coverage Diff @@
## master #5667 +/- ##
==============================================
- Coverage 87.05% 64.421% -22.63%
+ Complexity 31700 15093 -16607
==============================================
Files 1938 1204 -734
Lines 146097 70364 -75733
Branches 16128 11343 -4785
==============================================
- Hits 127178 45329 -81849
- Misses 13033 20294 +7261
+ Partials 5886 4741 -1145
|
return false; | ||
} | ||
if (genotype.hasDP() && genotype.getDP() == 0) { | ||
int[] pls = genotype.getPL(); |
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.
How about return MathUtils.max(genotype.getPL()) > 0
?
@Test | ||
public void testGenotypesWithNoDataDoNotChange() { | ||
final GenotypeBuilder gb = new GenotypeBuilder("s1"); | ||
gb.alleles(Arrays.asList(Allele.NO_CALL, Allele.NO_CALL)).DP(0).AD(new int[]{0,0}).GQ(0).PL(new int[]{0,0,0}); |
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.
How about chaining .alleles etc
with the previous line?
public void testGenotypesWithNoDataDoNotChange() { | ||
final GenotypeBuilder gb = new GenotypeBuilder("s1"); | ||
gb.alleles(Arrays.asList(Allele.NO_CALL, Allele.NO_CALL)).DP(0).AD(new int[]{0,0}).GQ(0).PL(new int[]{0,0,0}); | ||
final VariantContext vc = makeVC("noCall", Arrays.asList(Allele.create("A",true), Allele.create("T")), gb.make()); |
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.
Allele.create("A", true)
can be replaced with the pre-defined Allele.REF_A
.
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.
Just a few comments, no objection to the logic of the bug fix.
Back to you @davidbenjamin |
Good to merge! 👍 |
posterior probabilities and calls
Closes #5666