Shl edit samrecord docs

[sooheelee]

Clarify SAMRecord documentation so that users are not confused by thinking the opposite of what is meant.

• Three variable descriptions changed: getAlignmentStart, setAlignmentStart and getAlignmentEnd. Before, each read, "position of the clipped sequence," and now each reads, "position of the sequence remaining after clipping," for all three variable descriptions.
• In addition, took liberty to clarify "or 0 if there is no position" further. Now reads, "If no position, as would occur e.g. for the unmapped mate of a singly mapping pair, returns 0." Talked briefly to LB and agreed not to link to GATK documentation within htsjdk given it is a standalone project.

[sooheelee]

The commit with comments contain the actual documentation updates. The later commit (with no comments, sorry about that) just formats line spacing. Newbie here so please forgive.

[vdauwera]

The rephrasing looks good to me, much clearer. Regarding the formatting, I'm not sure but I thought the the "If no position" bit should remain on the same line to be parsed correctly by the javadoc system as being part of the @return element. @lbergelson can you advise?

[codecov-io]

Current coverage is 63.87% (diff: 100%)

Merging #782 into master will increase coverage by 0.03%

@@             master       #782   diff @@
==========================================
  Files           527        523     -4   
  Lines         31741      31642    -99   
  Methods           0          0          
  Messages          0          0          
  Branches       6788       6766    -22   
==========================================
- Hits          20263      20212    -51   
+ Misses         9335       9294    -41   
+ Partials       2143       2136     -7   

Powered by Codecov. Last update 7dbe733...46dcdd1

[nh13]

I am confused by the second line in each (singly mapped pair). I am not sure about the need for this change, but if you are adamant, then how about: 1-based inclusive leftmost position of the sequence remaining after clipping, or 0 if there is no position (ex. the read is unmapped)

[sooheelee]

@nh13, I believe the top of the SAMRecord doc talks about the types of unmapped reads that the GATK documentation refer to as part of a singly mapping pair here. I don't think the SAMRecord doc means to imply any unmapped read. My conclusion is that it must refer to those with coordinates within the SAM record but with * in the CIGAR field as happens for unmapped reads of singly mapping pairs.

Please correct if I'm mistaken.

If you feel strongly about merging the two sentences, and keeping it simple, then I can do:

1-based inclusive leftmost position of the sequence remaining after clipping, or 0 if there is no position, e.g. for an unmapped read in a singly mapping pair.

[julianhess]

It will return 0 for a totally unmapped read — it simply regurgitates what was present in the alignment record (e.g. see SAM decoding source here). Assuming the alignment conforms to SAM spec, a totally unmapped read will have RNAME = * and POS = 0; for an unmapped read whose mate is mapped, as @sooheelee notes, RNAME = <mate chromosome> and POS = 0. I agree with @nh13's more general phrasing.

[lbergelson]

@vdauwera The newlines don't make any difference to the doc output.

[nh13]

@sooheelee let me know when you make the change and I can look again.

[sooheelee]

@nh13 Now reads "1-based inclusive leftmost position of the sequence remaining after clipping, or 0 if there is no position, e.g. for unmapped read."

[sooheelee]

Thanks for the feedback @julianhess @nh13

[vdauwera]

Sounds like we have 👍 from everyone then?

[lbergelson]

Congrats @sooheelee on your first htsjdk commit!

[sooheelee]

Thank you @lbergelson!