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

Small CRAM refactor: common ExternalEncoding Abstract Base Class #1346

Merged
merged 2 commits into from
May 15, 2019

Conversation

jmthibault79
Copy link
Contributor

@jmthibault79 jmthibault79 commented Apr 3, 2019

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)

@@ -80,36 +77,36 @@ public CompressionHeader build(final List<CramCompressionRecord> records, Substi

final CompressionHeaderBuilder builder = new CompressionHeaderBuilder(coordinateSorted);

builder.addExternalIntegerRansOrderZeroEncoding(DataSeries.AP_AlignmentPositionOffset);
builder.addExternalByteRansOrderOneEncoding(DataSeries.BA_Base);
builder.addExternalRansOrderZeroEncoding(DataSeries.AP_AlignmentPositionOffset);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The typed versions (Byte, Integer, etc.) actually all did the same thing because their encoding parameters are identical. The Codecs differ, but that's not relevant here.

@codecov-io
Copy link

codecov-io commented Apr 3, 2019

Codecov Report

Merging #1346 into master will increase coverage by 0.29%.
The diff coverage is 100%.

@@              Coverage Diff               @@
##              master     #1346      +/-   ##
==============================================
+ Coverage     67.848%   68.139%   +0.29%     
- Complexity      8283      8448     +165     
==============================================
  Files            563       564       +1     
  Lines          33706     34302     +596     
  Branches        5657      5821     +164     
==============================================
+ Hits           22869     23373     +504     
- Misses          8659      8732      +73     
- Partials        2178      2197      +19
Impacted Files Coverage Δ Complexity Δ
...s/cram/encoding/external/ExternalLongEncoding.java 57.143% <100%> (-9.524%) 2 <1> (-1)
...tools/cram/encoding/external/ExternalEncoding.java 100% <100%> (ø) 2 <2> (?)
...s/cram/encoding/external/ExternalByteEncoding.java 100% <100%> (ø) 5 <1> (-1) ⬇️
.../samtools/cram/build/CompressionHeaderFactory.java 89.734% <100%> (-0.376%) 64 <0> (ø)
...ram/encoding/external/ExternalIntegerEncoding.java 100% <100%> (ø) 5 <1> (-1) ⬇️
...n/java/htsjdk/samtools/cram/structure/SliceIO.java 84.524% <0%> (+1.19%) 15% <0%> (+1%) ⬆️
...c/main/java/htsjdk/samtools/util/IntervalList.java 76.321% <0%> (+2.424%) 131% <0%> (+62%) ⬆️
...java/htsjdk/samtools/cram/structure/Container.java 91.781% <0%> (+3.737%) 31% <0%> (+4%) ⬆️
...samtools/util/AsyncBlockCompressedInputStream.java 76% <0%> (+4%) 13% <0%> (+1%) ⬆️
... and 4 more

@cmnbroad cmnbroad merged commit 5f0e045 into master May 15, 2019
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.

3 participants