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

Completes #5260 on GitHub. #5464

Merged
merged 4 commits into from
Nov 30, 2018
Merged

Completes #5260 on GitHub. #5464

merged 4 commits into from
Nov 30, 2018

Conversation

MartonKN
Copy link
Contributor

Changed SelectVariants so that it can handle multiple rsIDs separated by ';' in a VCF file.
The corresponding entry gets removed if any of those rsIDs has been set by the user to be deleted.
I also changed testExcludeSelectionIDFromFile() in SelectVariantsIntegrationTest to check this case.

…broke due to the change in the test file complexExample1.vcf. I modified the file testSelectVariants_KeepSelectionID.vcf appropriately so that the tests pass as they should.
Copy link
Contributor

@davidbenjamin davidbenjamin left a comment

Choose a reason for hiding this comment

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

Bak to @MartonKN

Copy link
Contributor Author

@MartonKN MartonKN left a comment

Choose a reason for hiding this comment

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

Made the requested changes -- thank you.

Copy link
Contributor

@davidbenjamin davidbenjamin left a comment

Choose a reason for hiding this comment

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

Couple more changes.

@davidbenjamin
Copy link
Contributor

@MartonKN Looks good! You can merge once tests pass.

@codecov-io
Copy link

Codecov Report

Merging #5464 into master will decrease coverage by 0.002%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #5464       +/-   ##
===============================================
- Coverage     86.986%   86.984%   -0.002%     
+ Complexity     31203     31201        -2     
===============================================
  Files           1909      1909               
  Lines         144195    144195               
  Branches       15951     15951               
===============================================
- Hits          125429    125426        -3     
- Misses         12997     13000        +3     
  Partials        5769      5769
Impacted Files Coverage Δ Complexity Δ
...bender/engine/filters/VariantIDsVariantFilter.java 100% <100%> (ø) 2 <1> (ø) ⬇️
...utils/smithwaterman/SmithWatermanIntelAligner.java 50% <0%> (-30%) 1% <0%> (-2%)
...ithwaterman/SmithWatermanIntelAlignerUnitTest.java 60% <0%> (ø) 2% <0%> (ø) ⬇️

@MartonKN MartonKN merged commit fb01bf3 into master Nov 30, 2018
@davidbenjamin
Copy link
Contributor

@MartonKN Don't forget to delete the branch. Note that a link to do so pops up when you merge a PR.

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.

4 participants