Allow the use of getSAMString on SAMReadGroupRecord objects #827

Merged
merged 2 commits into from Mar 24, 2017

Conversation

Projects
None yet
5 participants
Contributor

Lenbok commented Mar 16, 2017

Description

Fairly minor, this just allows programmatic access to the string representation of a read group, in much the same way as SAMRecord.getSAMString(). AFAIK there currently isn't really a non-convoluted way to get this. This is useful for displaying to the end user a problematic SAM read group.

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)
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2009 The Broad Institute
@nh13

nh13 Mar 16, 2017

Contributor

Did you build a time machine, out of a DeLorean? - 2009

@Lenbok

Lenbok Mar 17, 2017

Contributor

Haha! (Copied an existing test file from the same package as the starting point and the comment was folded in the editor off the bat! I can update if you want)

codecov-io commented Mar 16, 2017 edited

Codecov Report

Merging #827 into master will increase coverage by 0.002%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master      #827       +/-   ##
===============================================
+ Coverage     64.865%   64.868%   +0.002%     
- Complexity      7189      7194        +5     
===============================================
  Files            527       527               
  Lines          31769     31774        +5     
  Branches        5424      5424               
===============================================
+ Hits           20607     20611        +4     
+ Misses          9015      9014        -1     
- Partials        2147      2149        +2
Impacted Files Coverage Δ Complexity Δ
.../java/htsjdk/samtools/AbstractSAMHeaderRecord.java 57.143% <ø> (ø) 7 <0> (ø) ⬇️
.../main/java/htsjdk/samtools/SAMTextHeaderCodec.java 79.31% <100%> (+0.955%) 44 <4> (+3) ⬆️
src/main/java/htsjdk/samtools/SAMFileHeader.java 59.184% <100%> (+0.28%) 34 <0> (+1) ⬆️
...c/main/java/htsjdk/samtools/SAMSequenceRecord.java 68.293% <100%> (+0.391%) 31 <1> (+1) ⬆️
.../main/java/htsjdk/samtools/SAMReadGroupRecord.java 50% <100%> (+0.877%) 17 <1> (+1) ⬆️
...rc/main/java/htsjdk/samtools/SAMProgramRecord.java 75.676% <100%> (+6.231%) 15 <1> (+2) ⬆️
...sjdk/samtools/util/Md5CalculatingOutputStream.java 71.795% <0%> (-7.692%) 8% <0%> (-1%)
.../htsjdk/samtools/SAMRecordQueryNameComparator.java 40.741% <0%> (-7.407%) 7% <0%> (-2%)
...dk/samtools/seekablestream/SeekableHTTPStream.java 54.545% <0%> (-1.515%) 9% <0%> (-1%)
...a/htsjdk/samtools/cram/encoding/rans/Decoding.java 85.714% <0%> (+3.571%) 12% <0%> (+1%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c97c99...573d7ed. Read the comment docs.

@Lenbok rossb Allow the use of getSAMString on SAMReadGroupRecord objects
e77eb62

lbergelson was assigned by droazen Mar 21, 2017

Contributor

yfarjoun commented Mar 21, 2017

looks good @Lenbok. This is a good idea.

Could you do the same refactoring for the 3 other classes that inherit from AbstractSAMHeaderRecord and add an abstract getSAMString() function to AbstractSAMHeaderRecord?

Contributor

lbergelson commented Mar 21, 2017

@Lenbok This seems like a good idea and the implementation + test seems good. It seems like a good idea for all of the other types of header lines as well. If you don't mind, could you make toSAMString into an abstract method on AbstractSAMHeaderRecord and do analogous implementations for the other subclasses?

@Lenbok Lenbok Add getSAMString as abstract method to AbstractSAMHeaderRecord, imple…
…ment in subclasses and add extra tests.
573d7ed
Contributor

Lenbok commented Mar 24, 2017

@yfarjoun, @lbergelson No probs, done. I didn't construct an explicit new test for SAMFileHeader.getSAMString(), as that one is essentially refactored out of the existing clone() method, which already gets called all over the show.

Contributor

lbergelson commented Mar 24, 2017

@Lenbok Thanks you! 👍

@lbergelson lbergelson merged commit bbab32d into samtools:master Mar 24, 2017

3 of 4 checks passed

codecov/changes 3 files have unexpected coverage changes not visible in diff.
Details
codecov/patch 100% of diff hit (target 64.865%)
Details
codecov/project 64.868% (+0.002%) compared to 8c97c99
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment