Skip to content
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

Change VariantEval to not require the output file to already exist. #5681

Merged
merged 2 commits into from
Feb 19, 2019

Conversation

cmnbroad
Copy link
Collaborator

@cmnbroad cmnbroad commented Feb 14, 2019

Fixes #5674. All the tests are written using IntegrationTestSpec which creates the output files before running the test. Verified manually.

@codecov-io
Copy link

codecov-io commented Feb 14, 2019

Codecov Report

Merging #5681 into master will decrease coverage by 0.23%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##              master     #5681      +/-   ##
==============================================
- Coverage     87.043%   86.813%   -0.23%     
- Complexity     31707     31777      +70     
==============================================
  Files           1940      1940              
  Lines         146172    146804     +632     
  Branches       16130     16262     +132     
==============================================
+ Hits          127233    127445     +212     
- Misses         13051     13441     +390     
- Partials        5888      5918      +30
Impacted Files Coverage Δ Complexity Δ
...lbender/tools/walkers/varianteval/VariantEval.java 88.04% <ø> (-0.04%) 108 <0> (ø)
...aplotypecaller/HaplotypeCallerIntegrationTest.java 54.457% <0%> (-33.805%) 84% <0%> (ø)
...ls/CalculateGenotypePosteriorsIntegrationTest.java 65.169% <0%> (-29.913%) 13% <0%> (ø)
...iantutils/PosteriorProbabilitiesUtilsUnitTest.java 81.937% <0%> (-9.354%) 35% <0%> (-2%)
...titute/hellbender/engine/AssemblyRegionWalker.java 79.141% <0%> (-7.317%) 39% <0%> (+13%)
...oadinstitute/hellbender/engine/AssemblyRegion.java 80.808% <0%> (-3.688%) 65% <0%> (+17%)
...org/broadinstitute/hellbender/utils/MathUtils.java 77.108% <0%> (-2.492%) 238% <0%> (+18%)
...kers/variantutils/PosteriorProbabilitiesUtils.java 79.091% <0%> (-2.319%) 72% <0%> (+22%)
...ithwaterman/SmithWatermanIntelAlignerUnitTest.java 60% <0%> (ø) 2% <0%> (ø) ⬇️
...nder/utils/runtime/StreamingProcessController.java 67.773% <0%> (+0.474%) 33% <0%> (ø) ⬇️
... and 3 more

Copy link
Contributor

@droazen droazen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmnbroad Need one good test, then this can be merged

@@ -306,7 +306,6 @@ protected void initializeDrivingVariants() {
@Override
public void onTraversalStart() {
Utils.nonNull(outFile);
IOUtils.assertFileIsReadable(outFile.toPath());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add at least one test that would have failed without this patch?

@cmnbroad cmnbroad force-pushed the cn_variant_eval_outfile branch from b8be574 to 39aef2d Compare February 15, 2019 01:50
@cmnbroad cmnbroad merged commit ac1b146 into master Feb 19, 2019
@cmnbroad cmnbroad deleted the cn_variant_eval_outfile branch February 19, 2019 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants