/htsjdk

Fixed bug where SamPairUtil.setMateInfo() would omit the MQ tag if on… #647

Merged
merged 1 commit into from Jun 28, 2016

Conversation

Reviewers
No reviews
Assignees

@yfarjoun yfarjoun

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@tfenne @coveralls @yfarjoun @nh13
@tfenne
Owner

tfenne commented Jun 23, 2016

Description

Fix a small but annoying bug that was causing unmapped reads with mapped mates to get most mate information set except MQ which is set in all other cases.

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)

…e read was mapped and the other unmapped.
@tfenne
Owner

tfenne commented Jun 23, 2016

@ktibbett Want to take a quick look at this? This will effect FixMateInformation and MergeBamAlignment results, but in a good way.
@coveralls

coveralls commented Jun 23, 2016

Coverage Status

Coverage decreased (-0.004%) to 68.407% when pulling 3ec1736 on tf_fix_set_mate_info into 2e26fe8 on master.

@nh13 nh13 commented on an outdated diff Jun 23, 2016

View full outdated diff
src/main/java/htsjdk/samtools/SamPairUtil.java
@@ -246,12 +246,14 @@ else if (rec1.getReadUnmappedFlag() && rec2.getReadUnmappedFlag()) {
mapped.setMateUnmappedFlag(true);
// For the mapped read, set it's mateCigar to null, since the other read must be unmapped
mapped.setAttribute(SAMTag.MC.name(), null);
+ mapped.setAttribute(SAMTag.MQ.name(), null);
@nh13

nh13 Jun 23, 2016

Contributor

move to before setting the MC since that's the order elsewhere: it makes it easier to find.
@tfenne
Owner

tfenne commented Jun 27, 2016

@yfarjoun Want to return the favor and take a look here since @ktibbett seems busy?

@yfarjoun yfarjoun commented on an outdated diff Jun 28, 2016

View full outdated diff
src/main/java/htsjdk/samtools/SamPairUtil.java
mapped.setInferredInsertSize(0);
unmapped.setMateReferenceIndex(mapped.getReferenceIndex());
unmapped.setMateAlignmentStart(mapped.getAlignmentStart());
unmapped.setMateNegativeStrandFlag(mapped.getReadNegativeStrandFlag());
unmapped.setMateUnmappedFlag(false);
+ unmapped.setAttribute(SAMTag.MQ.name(), mapped.getMappingQuality());
// For the unmapped read, set it's mateCigar to the mate's Cigar, since the mate must be mapped
@yfarjoun

yfarjoun Jun 28, 2016

Contributor

I know it was like that before...can you remove the ' from "it's"
@yfarjoun
Contributor

yfarjoun commented Jun 28, 2016

👍 though I protest that this class lacks tests...we should at least open a ticket to record this fact...

yfarjoun was assigned by droazen Jun 28, 2016

@tfenne tfenne Fixed bug where SamPairUtil.setMateInfo() would omit the MQ tag if on… 
…e read was mapped and the other unmapped.

Some checks were not successful

1 failing and 2 successful checks
coverage/coveralls — Coverage decreased (-0.008%) to 68.405%
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
4b22d5d
@tfenne
Owner

tfenne commented Jun 28, 2016

Updates applied. @yfarjoun If we logged issues for everything that needed testing... we'd have a lot of issues 😞 .
@coveralls

coveralls commented Jun 28, 2016

Coverage Status

Coverage decreased (-0.008%) to 68.405% when pulling 4b22d5d on tf_fix_set_mate_info into 7084c88 on master.
@yfarjoun
Contributor

yfarjoun commented Jun 28, 2016

👍

@tfenne tfenne merged commit d683012 into master Jun 28, 2016
2 of 3 checks passed

2 of 3 checks passed

coverage/coveralls Coverage decreased (-0.008%) to 68.405%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

tfenne deleted the tf_fix_set_mate_info branch Jun 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment