ignore TC, TN on CRAM read

[yash-puligundla]

Description

As per CRAMv3,

TC and TN are legacy data series from CRAM 1.0. They have no function in CRAM 3.0 and should not be present. However, some implementations do output them and decoders must silently skip these fields. It is illegal for TC and TN to contain any data values, although there may be empty blocks associated with them. TC, TN data series should not be written by cram writer. However, if they are present, they should be ignored

More details here

Fix

• Remove TC, TN from CompressionHeaderEncodingMap.java
• Add TC, TN to the list of data series being ignored
• there are pre-existing tests that check that the data series that should be ignored are being ignored and are always null.

[codecov-commenter]

Codecov Report

Merging #1578 (d56e3d8) into master (a4f1f04) will decrease coverage by 0.053%. The diff coverage is 100.000%.

:exclamation: Current head d56e3d8 differs from pull request most recent head 09619d1. Consider uploading reports for the commit 09619d1 to get more accurate results

@@               Coverage Diff               @@
##              master     #1578       +/-   ##
===============================================
- Coverage     69.839%   69.786%   -0.053%     
  Complexity      9638      9638               
===============================================
  Files            702       702               
  Lines          37618     37516      -102     
  Branches        6111      6074       -37     
===============================================
- Hits           26272     26181       -91     
+ Misses          8900      8881       -19     
- Partials        2446      2454        +8     

Impacted Files	Coverage Δ
...ava/htsjdk/samtools/cram/structure/DataSeries.java	100.000% <ø> (ø)
...s/cram/structure/CompressionHeaderEncodingMap.java	85.621% <100.000%> (+0.486%)	:arrow_up:
...htsjdk/samtools/util/nio/DeleteOnExitPathHook.java	80.952% <0.000%> (-9.524%)	:arrow_down:
...sjdk/samtools/util/htsget/HtsgetErrorResponse.java	73.333% <0.000%> (-6.667%)	:arrow_down:
...tools/cram/encoding/core/GammaIntegerEncoding.java	86.667% <0.000%> (-6.667%)	:arrow_down:
.../samtools/cram/compression/ExternalCompressor.java	69.565% <0.000%> (-4.509%)	:arrow_down:
...mtools/cram/encoding/core/BetaIntegerEncoding.java	91.304% <0.000%> (-4.348%)	:arrow_down:
...am/encoding/core/CanonicalHuffmanByteEncoding.java	52.941% <0.000%> (-2.941%)	:arrow_down:
...ing/core/huffmanUtils/HuffmanParamsCalculator.java	80.000% <0.000%> (-1.818%)	:arrow_down:
...va/htsjdk/samtools/util/htsget/HtsgetResponse.java	89.394% <0.000%> (-1.783%)	:arrow_down:
... and 41 more

[yash-puligundla]

• I have added a test to inspect the CompressionHeaderEncodingMap and assert that obsolete data series, TC and TN are not present.
• made the recommended changes to the comments and added links to the related hts-specs issues.
• DATASERIES_NOT_WRITTEN_BY_HTSJDK and DATASERIES_NOT_READ_BY_HTSJDK are defined only in StructureTestUtils.

[cmnbroad]

@yash-puligundla Also, you might try wrapping both of the two data series sets (DATASERIES_NOT_WRITTEN_BY_HTSJDK and DATASERIES_NOT_READ_BY_HTSJDK) with Collections.unmodifiableSet - I think that will fix the spotBugs issue. Not sure why it only complains about one of them.

[yash-puligundla]

@yash-puligundla Also, you might try wrapping both of the two data series sets (DATASERIES_NOT_WRITTEN_BY_HTSJDK and DATASERIES_NOT_READ_BY_HTSJDK) with Collections.unmodifiableSet - I think that will fix the spotBugs issue. Not sure why it only complains about one of them.

I will do that. Thanks!

[yash-puligundla]

• Added a roundtrip test
• made other suggested changes
• changed the HashSet into Collections.unmodifiableSet LinkedHashSet

NOTE: We can't truly roundtrip and test for the absence of TC, TN as we have added code to ignore TC, TN on CRAM read. However, following @cmnbroad 's suggestion, I ran the tests -

• by removing the code changes made both for ignoring TC, TN on read and not writing TC, TN. In this case, the test would fail as TC and TN are not null.

• by removing just the code change that ignores TC, TN and inspecting the encoding map. The encoding map of the roundtripped file does not have TC, TN in it.

[cmnbroad]

@lbergelson Can you merge this one ? thanks.

[lbergelson]

@yash-puligundla @cmnbroad Thank you!