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

Support writing a CRAI index from CRAMContainerStreamWriter #1351

Merged
merged 2 commits into from
May 20, 2019

Conversation

tomwhite
Copy link
Contributor

@tomwhite tomwhite commented Apr 8, 2019

Description

This is to make it possible to write CRAI indexes from CRAMContainerStreamWriter, not just BAI indexes (as is the case currently). This is needed by disq-bio/disq#97.

This PR introduces AbstractCRAMIndexer that is the common base class of CRAMBAIIndexer and CRAMCRAIIndexer; and adds a constructor to CRAMContainerStreamWriter that takes an AbstractCRAMIndexer.

Checklist

  • Code compiles correctly
  • New tests covering changes and new functionality
  • All tests passing
  • Extended the README / documentation, if necessary
  • Is not backward compatible (breaks binary or source compatibility)

@codecov-io
Copy link

codecov-io commented Apr 8, 2019

Codecov Report

Merging #1351 into master will increase coverage by 0.013%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #1351       +/-   ##
===============================================
+ Coverage     67.848%   67.861%   +0.013%     
- Complexity      8283      8286        +3     
===============================================
  Files            563       563               
  Lines          33706     33710        +4     
  Branches        5657      5656        -1     
===============================================
+ Hits           22869     22876        +7     
+ Misses          8659      8657        -2     
+ Partials        2178      2177        -1
Impacted Files Coverage Δ Complexity Δ
src/main/java/htsjdk/samtools/CRAMBAIIndexer.java 81.034% <ø> (ø) 17 <0> (ø) ⬇️
...ava/htsjdk/samtools/CRAMContainerStreamWriter.java 72.458% <100%> (+0.117%) 55 <2> (+1) ⬆️
src/main/java/htsjdk/samtools/CRAMCRAIIndexer.java 81.818% <100%> (+0.866%) 10 <1> (+1) ⬆️
...antcontext/writer/VariantContextWriterBuilder.java 82.803% <0%> (+0.11%) 60% <0%> (ø) ⬇️
...samtools/util/AsyncBlockCompressedInputStream.java 76% <0%> (+4%) 13% <0%> (+1%) ⬆️

@tomwhite
Copy link
Contributor Author

tomwhite commented Apr 8, 2019

cc @cmnbroad

@cmnbroad cmnbroad added the cram label Apr 8, 2019
Copy link
Collaborator

@cmnbroad cmnbroad left a comment

Choose a reason for hiding this comment

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

Thx for making the common class/interface. Looks pretty good - just a request to factor out some duplicated test code.

@tomwhite
Copy link
Contributor Author

Thanks for all the feedback @pshapiro4broad and @cmnbroad. I've addressed all the points in a new commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants