samtools / htsjdk

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

- Make get/set attributes methods take a SAMTag input #1472

Closed yfarjoun closed 4 years ago

yfarjoun commented 4 years ago

... so that callers do not need to adorn their code with useless .name() or .getBinaryTag() calls.

Description

Please explain the changes you made here. Explain the motivation for making this change. What existing problem does the pull request solve?

Things to think about before submitting:

yfarjoun commented 4 years ago

@pshapiro4broad you inspired me to do this....

lbergelson commented 4 years ago

Oh man, I've wanted to do this for a long time but I've been too lazy.

lbergelson commented 4 years ago

Oh man, I've wanted to do this for a long time but I've been too lazy. The thing we MIGHT want to do is make it return a typed value when given a tag with a known type. Thats more work though.

yfarjoun commented 4 years ago

wouldn't that require having separate enums for the different types?

On Wed, Apr 15, 2020 at 11:18 AM Louis Bergelson notifications@github.com wrote:

Oh man, I've wanted to do this for a long time but I've been too lazy. The thing we MIGHT want to do is make it return a typed value when given a tag with a known type. Thats more work though.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/samtools/htsjdk/pull/1472#issuecomment-614102479, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAU6JURIVNIZJOLQQXX3ROLRMXF55ANCNFSM4MIU37SA .

lbergelson commented 4 years ago

yes, until http://openjdk.java.net/jeps/301 releases in ???

lbergelson commented 4 years ago

@yfarjoun This is waiting on an update to the comment about avoiding redundant conversion in SAMRecord.hasAttribute and friends.

codecov-commenter commented 4 years ago

Codecov Report

Merging #1472 into master will increase coverage by 0.391%. The diff coverage is 60.000%.

@@               Coverage Diff               @@
##              master     #1472       +/-   ##
===============================================
+ Coverage     69.205%   69.596%   +0.391%     
- Complexity      8700      9066      +366     
===============================================
  Files            587       601       +14     
  Lines          34580     36212     +1632     
  Branches        5779      6088      +309     
===============================================
+ Hits           23931     25202     +1271     
- Misses          8367      8664      +297     
- Partials        2282      2346       +64     
Impacted Files Coverage Δ Complexity Δ
src/main/java/htsjdk/samtools/SAMUtils.java 62.802% <33.333%> (ø) 131.000 <1.000> (ø)
...c/main/java/htsjdk/samtools/util/SequenceUtil.java 70.118% <33.333%> (ø) 130.000 <18.000> (ø)
src/main/java/htsjdk/samtools/SAMRecord.java 67.433% <41.667%> (-0.890%) 303.000 <11.000> (+6.000) :arrow_down:
...main/java/htsjdk/samtools/SAMRecordSetBuilder.java 89.591% <68.750%> (ø) 75.000 <0.000> (ø)
src/main/java/htsjdk/samtools/SamPairUtil.java 57.488% <70.588%> (ø) 20.000 <0.000> (ø)
.../java/htsjdk/samtools/AbstractSAMHeaderRecord.java 75.000% <100.000%> (+3.571%) 9.000 <1.000> (+1.000)
.../htsjdk/samtools/SAMRecordQueryNameComparator.java 92.593% <100.000%> (ø) 21.000 <0.000> (ø)
...rc/main/java/htsjdk/samtools/SamFileValidator.java 84.211% <100.000%> (ø) 82.000 <3.000> (ø)
.../main/java/htsjdk/samtools/fastq/FastqEncoder.java 89.744% <100.000%> (ø) 17.000 <1.000> (ø)
src/main/java/htsjdk/samtools/util/CigarUtil.java 53.571% <100.000%> (ø) 4.000 <0.000> (ø)
... and 40 more