Don't try to index BAM files that can't be indexed with HTSJDK. #520
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #520 +/- ##
==========================================
+ Coverage 95.06% 95.06% +<.01%
==========================================
Files 102 102
Lines 5770 5774 +4
Branches 411 401 -10
==========================================
+ Hits 5485 5489 +4
Misses 285 285
Continue to review full report at Codecov.
|
nh13
approved these changes
Aug 19, 2019
| val hasChromsTooLong = header.getSequenceDictionary.getSequences.exists(_.getSequenceLength > GenomicIndexUtil.BIN_GENOMIC_SPAN) | ||
| val wouldHaveIndexed = header.getSortOrder == SortOrder.coordinate && index | ||
| if (wouldHaveIndexed && hasChromsTooLong) logger.warning(s"Cannot index $path as one or more chromosomes is too long.") | ||
| wouldHaveIndexed && !hasChromsTooLong |
There was a problem hiding this comment.
if (header.getSortOrder != SortOrder.coordinate || !index) false else {
val hasChromsTooLong = header.getSequenceDictionary.getSequences.forall(_.getSequenceLength <= GenomicIndexUtil.BIN_GENOMIC_SPAN)
if (hasChromsTooLong) logger.warning(s"Cannot index $path as one or more chromosomes is too long.")
hasChromsTooLong
}
| @@ -95,6 +95,7 @@ lazy val commonSettings = Seq( | |||
| fork in Test := true, | |||
| resolvers += Resolver.sonatypeRepo("public"), | |||
| resolvers += Resolver.mavenLocal, | |||
| resolvers += "broad-snapshots" at "https://artifactory.broadinstitute.org/artifactory/libs-snapshot/", | |||
There was a problem hiding this comment.
is this to get htsjdk snapshots? Sucks, but I understand.
There was a problem hiding this comment.
It is indeed for htsjdk snapshots.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@nh13 The change here is quite simple. HTSJDK blows up if you ask it to index on the fly and hand it reference sequences that are beyond what BAI can handle. There's no CSI writing support yet either.
I'd also like to wait until samtools/htsjdk#1410 is merged, and update our HTSJDK dependency in this same PR if possible.