Added alignment type to RecordAndOffset class #1411
Conversation
- as requested per Picard pull request #1370 - necessary to determine whether a RecordAndOffset object is an (alignment) match, insertion, or deletion when queried by a SamLocusIterator - default behavior is preserved by overloading the default constructor
There was a problem hiding this comment.
@michaelgatzen The code looks good to me. Could you add a test in SamLocusIterator that shows that this information is correctly associated with insertions / deletions? You might be able to just add an extra assertion to existing tests for insertion/deletion functionality. Even though this is pretty simple, it's good to have a defensive check in case someone breaks it accidentally in the future.
Also, as an aside, if you include an issue number for another repository in the commit / github messages you should use a full link rather than a number, because github will erroneously link a simple issue number to the htsjdk issue if you don't.
|
@lbergelson Agreed, I added lots of asserts in |
| @@ -582,6 +658,10 @@ public void testOverlappingGappedAlignmentsWithoutIndels() { | |||
| Assert.assertEquals(li.size(), expectedDepths[i]); | |||
| Assert.assertEquals(li.getPosition(), expectedReferencePositions[i]); | |||
| Assert.assertEquals(li.getRecordAndOffsets().size(), expectedReadOffsets[i].length); | |||
| // Check the correct assignment of the alignment type | |||
| for (final SamLocusIterator.RecordAndOffset rao : li.getRecordAndOffsets()) { | |||
There was a problem hiding this comment.
This is all a bit redundant, but it just seems like these tests have a ton of redundant code. We can clean that up separately.
|
@michaelgatzen Thank you and congrats on your first htsjdk pr :) |
(alignment) match, insertion, or deletion when queried by a
SamLocusIterator
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: