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

Public method for CRAMIntervalIterator for overlapping intervals #1120

Merged

Conversation

tomwhite
Copy link
Contributor

Description

Expose a method to create a CRAMIntervalIterator to get reads overlapping intervals, much like BAMFileReader#createIndexIterator. See #1112

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 May 22, 2018

Codecov Report

Merging #1120 into master will increase coverage by 0.011%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #1120       +/-   ##
===============================================
+ Coverage     66.052%   66.063%   +0.011%     
- Complexity      7685      7690        +5     
===============================================
  Files            538       538               
  Lines          32582     32584        +2     
  Branches        5529      5529               
===============================================
+ Hits           21521     21526        +5     
+ Misses          8909      8907        -2     
+ Partials        2152      2151        -1
Impacted Files Coverage Δ Complexity Δ
src/main/java/htsjdk/samtools/CRAMFileReader.java 74.5% <100%> (+0.258%) 53 <4> (+4) ⬆️
...samtools/util/AsyncBlockCompressedInputStream.java 76% <0%> (+4%) 13% <0%> (+1%) ⬆️

@@ -466,6 +466,32 @@ void enableFileSource(final SamReader reader, final boolean enabled) {
iterator.setFileSource(enabled ? reader : null);
}

/**
* Prepare to iterate through SAMRecords that match the given intervals.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should slightly narrow the description of what this method does. Since there is no validation that the provided chunks actually match (fully span) the territory covered by the intervals, it possible that the iterator doesn't return the full set of records matching the query. The records are constrained first by the chunks, then further constrained by the intervals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've revised the wording to say "Prepare to iterate through SAMRecords that match the intersection of the given intervals and chunk boundaries."

@yfarjoun yfarjoun added the cram label Jun 4, 2018
Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

@tomwhite This looks good to me, it just needs a test for the new method.

…ping intervals, much like BAMFileReader#createIndexIterator.
@tomwhite tomwhite force-pushed the public_cram_overlapping_intervals branch from 5539d81 to 92b4342 Compare June 7, 2018 10:49
@tomwhite
Copy link
Contributor Author

tomwhite commented Jun 7, 2018

Added a new test.

@lbergelson
Copy link
Member

@tomwhite Thank you.

@lbergelson lbergelson merged commit 9ea0a03 into samtools:master Jun 15, 2018
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.

None yet

5 participants