search

samtools / htsjdk

A Java API for high-throughput sequencing data (HTS) formats.
http://samtools.github.io/htsjdk/
283 stars 242 forks source link

Make VariantContextWriterBuilder warn when indexing-on-the-fly is enabled for stream output #1328

Closed jamesemery closed 5 years ago

jamesemery commented 5 years ago

This is the first stage in resolving https://github.com/broadinstitute/gatk/issues/5779. Currently in when we provide a stream (or any non-regular file to resolve) to the VariantContextWriterBuilder we set .setOutputFileType(OutputType.VCF). This sidesteps the code in the switch statement designed to this case and throw an exception if indexing on the fly is enabled. It turns out to be a pain point for the users because indexing on the fly is enabled by default. I propose that we push the responsibility of deciding if the output is a stream up to gatk, and then make the stream code in VariantContextWriterBuilder consistent with the BAM writer, which only warns and disables indexing if it suspects the file might be a stream.

jamesemery commented 5 years ago

@cmnbroad

codecov-io commented 5 years ago

Codecov Report

Merging #1328 into master will increase coverage by 0.17%. The diff coverage is 40%.

@@              Coverage Diff               @@
##              master     #1328      +/-   ##
==============================================
+ Coverage     67.822%   67.992%   +0.17%     
- Complexity      8258      8344      +86     
==============================================
  Files            562       570       +8     
  Lines          33663     33826     +163     
  Branches        5644      5655      +11     
==============================================
+ Hits           22831     22999     +168     
+ Misses          8658      8645      -13     
- Partials        2174      2182       +8
Impacted Files Coverage Δ Complexity Δ
...antcontext/writer/VariantContextWriterBuilder.java 80.625% <40%> (-2.067%) 59 <0> (-1)
...s/cram/encoding/external/ExternalLongEncoding.java 57.143% <0%> (-9.524%) 2% <0%> (-1%)
src/main/java/htsjdk/variant/bcf2/BCFVersion.java 88.889% <0%> (-5.848%) 11% <0%> (+4%)
...amtools/reference/FastaReferenceWriterBuilder.java 79.518% <0%> (-4.118%) 38% <0%> (+13%)
...java/htsjdk/samtools/cram/structure/Container.java 88.043% <0%> (-2.433%) 27% <0%> (+5%)
...c/main/java/htsjdk/samtools/util/IntervalList.java 72.18% <0%> (-2.329%) 64% <0%> (-11%)
...samtools/cram/build/CramSpanContainerIterator.java 71.053% <0%> (-2.118%) 9% <0%> (ø)
...tsjdk/variant/variantcontext/writer/VCFWriter.java 81.72% <0%> (-1.234%) 19% <0%> (+1%)
...jdk/samtools/cram/build/CramContainerIterator.java 88.889% <0%> (-1.111%) 10% <0%> (ø)
src/main/java/htsjdk/samtools/util/Interval.java 63.333% <0%> (-0.952%) 28% <0%> (+1%)
... and 51 more