-
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
Fixed Concordance tool to allow for no variation alleles in the truth data. #5718
Fixed Concordance tool to allow for no variation alleles in the truth data. #5718
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5718 +/- ##
===============================================
- Coverage 87.069% 87.067% -0.003%
- Complexity 31875 31880 +5
===============================================
Files 1940 1940
Lines 146738 146756 +18
Branches 16226 16229 +3
===============================================
+ Hits 127764 127776 +12
- Misses 13061 13065 +4
- Partials 5913 5915 +2
|
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.
Minor comments. You can merge without sending it back.
// we assume that the truth has a single alternate allele | ||
final boolean containsAltAllele = eval.getAlternateAlleles().contains(truth.getAlternateAllele(0)); | ||
final boolean containsAltAllele = |
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.
You need to update the comment on line 294.
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.
Fixed!
|
||
@Test | ||
public void testDoesNotCrashWithNO_VARIATIONAlleles() { | ||
final String testDir = toolsTestDir + "concordance/"; |
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.
This appears elsewhere in this test class, so perhaps extract a static constant for it.
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.
Sounds good! Fixed!
runCommandLine(args); | ||
|
||
try { | ||
final ConcordanceSummaryRecord.Reader reader = new ConcordanceSummaryRecord.Reader(summary); |
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.
There's some strange spacing here.
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.
Ah. I didn't reformat the code before I checked it in. I like to align my blocks of variable definitions.
Fixed!
Fixed Concordance tool to allow for no variation alleles in the truth data.