Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fixed an error where the read mate key in SplitNCigarReads was not sufficiently strict about readnames causing cigar errors #6909

Merged
merged 5 commits into from
Nov 16, 2020
Merged

fixed an error where the read mate key in SplitNCigarReads was not sufficiently strict about readnames causing cigar errors #6909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d9bb691
fixed an error where the read mate key in SplitNCigarReads was not su…
jamesemery Oct 23, 2020
4e2af3b
more poorly understood magic back to bite us
jamesemery Oct 26, 2020
a57b36c
more evil magic will keep sauron at bay
jamesemery Oct 26, 2020
3e31d07
removing sudos
jamesemery Oct 29, 2020
034e18e
and those other ones
jamesemery Oct 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/main/java/org/broadinstitute/hellbender/tools/walkers/rnaseq/OverhangFixingManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,11 @@ public void setMateChanged() {
}
}

// Generates the string key to be used for
private static String makeKey(String name, boolean firstOfPair, int mateStart) {
return name + (firstOfPair ? 1 : 0) + mateStart;
// Generates the string key to be used for storing mates that had their cigar strings changed between iterations over the input reads
@VisibleForTesting
static String makeKey(String name, boolean firstOfPair, int mateStart) {
// NOTE: We use the '@' character here because it is explicitly forbidden for use in read names by the SAM Spec and we don't want to risk accidental matches across read pairs
return name + "@" + (firstOfPair ? 1 : 0) + "@" + mateStart;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}
/**
* Will edit the mate MC tag and mate start position field for given read if that read has been recorded being edited
Expand Down
14 changes: 14 additions & 0 deletions ...ava/org/broadinstitute/hellbender/tools/walkers/rnaseq/OverhangFixingManagerUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,20 @@ public void testMappingReadMateRepair() {
Assert.assertEquals(read2.getMateStart(), 10003);
}

@Test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamesemery Have you tested what happens if you add a read with a @s in the name which could recreate the problem?

// Test asserting that the failure found in https://github.com/broadinstitute/gatk/issues/6776 has been fixed
public void testIncidentalMatchFromReportedFailure() {
GATKRead read1 = ArtificialReadUtils.createArtificialRead(hg19Header, "RR.6123", 1, 3500, 100);
read1.setIsFirstOfPair();
GATKRead read2 = ArtificialReadUtils.createArtificialUnmappedRead(hg19Header, new byte[]{(byte)'A'}, new byte[]{(byte)'A'});
read2.setName("RR.6123135");
read2.setIsSecondOfPair();

// Asserting that in the case of reads that end with widely ranged numbers it is no longer possible to incidentally overlap in key generation
Assert.assertNotEquals( OverhangFixingManager.makeKey(read1.getName(), read1.isFirstOfPair(), read1.getStart()),
OverhangFixingManager.makeKey(read2.getName(), read2.isFirstOfPair(), read2.getStart()));
}

@Test
public void testMappingReadMateRepairNoMCTag() {
final OverhangFixingManager manager = new OverhangFixingManagerAlwaysSplit10000Reads(getHG19Header(), null, hg19GenomeLocParser, hg19ReferenceReader, 10000, 1, 40, false, false);
Expand Down