Added setUnplaced() to the GATKRead api to distinguish reads with no mapping information #5320
Conversation
|
@lbergelson Can i get a review on this branch? |
There was a problem hiding this comment.
@jamesemery A bunch of misc documentation change requests. I think it's sane though.
| /** | ||
| * Reverse-complement bases and reverse quality scores along with known optional attributes that | ||
| * need the same treatment. Changes made after making a copy of the bases, qualities, | ||
| * and any attributes that will be altered. If in-place update is needed use |
There was a problem hiding this comment.
Maybe something like "Modify this read by reverse complementing its bases and reversing its quality scores. Implementations may also update tags that are known to need updating after the reverse complement operation."
Leave out the stuff about in place update.
| @@ -469,6 +469,20 @@ default int numCigarElements(){ | |||
| */ | |||
| void setIsUnmapped(); | |||
|
|
|||
| /** | |||
| * @return True if this read is unmapped, not including reads where the position is set but the read is marked as unmapped. Otherwise false. | |||
There was a problem hiding this comment.
Maybe something like this would be slightly more clear?
| * @return True if this read is unmapped, not including reads where the position is set but the read is marked as unmapped. Otherwise false. | |
| * Does the read have a position assigned to it for sorting purposes. | |
| * @return `true iff this read has no assigned position or contig. |
| /** | ||
| * @return True if this read's mate is unmapped, not including reads where the position is set but the read is marked as unmapped. Otherwise false. | ||
| */ | ||
| boolean mateIsUnplaced(); |
There was a problem hiding this comment.
provide a default implementation
There was a problem hiding this comment.
Unfortunately I can't provide a default implementation here because getContig() masks empty values. The whole point of this PR was to provide better access to the contig index and start position in samrecord beyond what getContig(), and getStart() currently provide.
There was a problem hiding this comment.
you're right that there isn't a getMateAssignedContig. Should there be?
| /** | ||
| * @return True if this read is unmapped, not including reads where the position is set but the read is marked as unmapped. Otherwise false. | ||
| */ | ||
| boolean isUnplaced(); |
There was a problem hiding this comment.
I would provide a default implementation of this method.
There was a problem hiding this comment.
same response as above
There was a problem hiding this comment.
can't a default implementation of this one use getAssignedContig?
| boolean isUnplaced(); | ||
|
|
||
| /** | ||
| * Mark the read as unmapped, and also removes mapping information from the read (i.e. sets contig to "*" and position to 0). |
There was a problem hiding this comment.
| * Mark the read as unmapped, and also removes mapping information from the read (i.e. sets contig to "*" and position to 0). | |
| * Mark the read as unmapped, and also removes mapping information from the read (i.e. sets contig to {@link ReadConstants#UNSET_CONTIG} and position to {@link ReadConstants#UNSET_POSITION}). |
| * Calling this method has the additional effect of marking the read as paired, as if {@link #setIsPaired} | ||
| * were invoked with true. | ||
| */ | ||
| void setMateIsUnplaced(); |
|
@lbergelson Resonded to your comments |
Codecov Report
@@ Coverage Diff @@
## master #5320 +/- ##
==============================================
+ Coverage 86.793% 87.09% +0.296%
- Complexity 30107 30493 +386
==============================================
Files 1842 1854 +12
Lines 139393 141474 +2081
Branches 15369 15530 +161
==============================================
+ Hits 120984 123209 +2225
+ Misses 12823 12618 -205
- Partials 5586 5647 +61
|
Relates to my work on #4461