From aa04ef75d06f0fb1df7a92ff1bd01d29c6af131c Mon Sep 17 00:00:00 2001 From: meganshand Date: Thu, 29 Sep 2016 15:51:12 -0400 Subject: [PATCH 1/6] Includes broken test for reverseComplementing without a deep copy --- src/main/java/htsjdk/samtools/SAMRecord.java | 3 +- src/main/java/htsjdk/samtools/SAMRecordUtil.java | 54 +++++++++++++++++----- .../java/htsjdk/samtools/SAMRecordUtilTest.java | 50 ++++++++++++++++---- 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/src/main/java/htsjdk/samtools/SAMRecord.java b/src/main/java/htsjdk/samtools/SAMRecord.java index a22ded540..24ae3086d 100644 --- a/src/main/java/htsjdk/samtools/SAMRecord.java +++ b/src/main/java/htsjdk/samtools/SAMRecord.java @@ -2112,7 +2112,8 @@ private String buildMessage(final String baseMessage, final boolean isMate) { /** * Note that this does a shallow copy of everything, except for the attribute list, for which a copy of the list * is made, but the attributes themselves are copied by reference. This should be safe because callers should - * never modify a mutable value returned by any of the get() methods anyway. + * never modify a mutable value returned by any of the get() methods anyway. If one of the cloned records SEQ or + * QUAL needs to be modified, a deeper copy should be made, or other precautions taken (Eg. Reverse Complement). */ @Override public Object clone() throws CloneNotSupportedException { diff --git a/src/main/java/htsjdk/samtools/SAMRecordUtil.java b/src/main/java/htsjdk/samtools/SAMRecordUtil.java index d2900747f..c9bfac021 100644 --- a/src/main/java/htsjdk/samtools/SAMRecordUtil.java +++ b/src/main/java/htsjdk/samtools/SAMRecordUtil.java @@ -39,11 +39,26 @@ /** * Reverse-complement bases and reverse quality scores along with known optional attributes that - * need the same treatment. See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} + * need the same treatment. This does not make a deep copy of the bases, qualities or attributes: + * for safe reverseComplement see {@link #reverseComplement(SAMRecord, boolean)}. + * See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} * for the default set of tags that are handled. */ public static void reverseComplement(final SAMRecord rec) { - reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE); + reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, true); + } + + /** + * Reverse-complement bases and reverse quality scores along with known optional attributes that + * need the same treatment. Optionally makes a deep copy of the bases, qualities or attributes. + * See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} + * for the default set of tags that are handled. + * + * @param rec Record to reverse complement. + * @param unsafe Setting this to false will clone all attributes, bases and qualities before changing the values. + */ + public static void reverseComplement(final SAMRecord rec, boolean unsafe) { + reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, unsafe); } /** @@ -51,11 +66,11 @@ public static void reverseComplement(final SAMRecord rec) { * non-null attributes specified by tagsToRevcomp and reverse and non-null attributes * specified by tagsToReverse. */ - public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse) { - final byte[] readBases = rec.getReadBases(); + public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse, boolean unsafe) { + final byte[] readBases = unsafe ? rec.getReadBases() : rec.getReadBases().clone(); SequenceUtil.reverseComplement(readBases); rec.setReadBases(readBases); - final byte qualities[] = rec.getBaseQualities(); + final byte qualities[] = unsafe ? rec.getBaseQualities() : rec.getBaseQualities().clone(); reverseArray(qualities); rec.setBaseQualities(qualities); @@ -64,8 +79,13 @@ public static void reverseComplement(final SAMRecord rec, final Collection Date: Thu, 29 Sep 2016 21:48:13 -0400 Subject: [PATCH 2/6] Fixes test --- src/test/java/htsjdk/samtools/SAMRecordUtilTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/htsjdk/samtools/SAMRecordUtilTest.java b/src/test/java/htsjdk/samtools/SAMRecordUtilTest.java index a8e808ff4..fe23c826a 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUtilTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUtilTest.java @@ -24,8 +24,8 @@ @Test public void testSafeReverseComplement() throws CloneNotSupportedException { final SAMRecord original = createTestSamRec(); final SAMRecord cloneOfOriginal = (SAMRecord) original.clone(); - //Runs an unsafe reverseComplement - SAMRecordUtil.reverseComplement(cloneOfOriginal, Arrays.asList("Y1"), Arrays.asList("X1", "X2", "X3", "X4", "X5"), true); + //Runs a safe reverseComplement + SAMRecordUtil.reverseComplement(cloneOfOriginal, Arrays.asList("Y1"), Arrays.asList("X1", "X2", "X3", "X4", "X5"), false); Assert.assertEquals(original.getReadString(), "ACACACACAC"); Assert.assertEquals(original.getBaseQualityString(), "HHHHHIIIII"); From 5c3b01ef3f8a6c821f11fe6509d825be9f25bd0c Mon Sep 17 00:00:00 2001 From: meganshand Date: Tue, 4 Oct 2016 16:44:22 -0400 Subject: [PATCH 3/6] Addressing most comments --- src/main/java/htsjdk/samtools/SAMRecord.java | 4 +-- src/main/java/htsjdk/samtools/SAMRecordUtil.java | 31 +++++++++++----------- .../java/htsjdk/samtools/SAMRecordUtilTest.java | 4 +-- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/main/java/htsjdk/samtools/SAMRecord.java b/src/main/java/htsjdk/samtools/SAMRecord.java index 24ae3086d..7b6b45e58 100644 --- a/src/main/java/htsjdk/samtools/SAMRecord.java +++ b/src/main/java/htsjdk/samtools/SAMRecord.java @@ -2112,8 +2112,8 @@ private String buildMessage(final String baseMessage, final boolean isMate) { /** * Note that this does a shallow copy of everything, except for the attribute list, for which a copy of the list * is made, but the attributes themselves are copied by reference. This should be safe because callers should - * never modify a mutable value returned by any of the get() methods anyway. If one of the cloned records SEQ or - * QUAL needs to be modified, a deeper copy should be made, or other precautions taken (Eg. Reverse Complement). + * never modify a mutable value returned by any of the get() methods anyway. If one of the cloned record's SEQ or + * QUAL needs to be modified, a deeper copy should be made (e.g. Reverse Complement). */ @Override public Object clone() throws CloneNotSupportedException { diff --git a/src/main/java/htsjdk/samtools/SAMRecordUtil.java b/src/main/java/htsjdk/samtools/SAMRecordUtil.java index c9bfac021..8c4ac44e0 100644 --- a/src/main/java/htsjdk/samtools/SAMRecordUtil.java +++ b/src/main/java/htsjdk/samtools/SAMRecordUtil.java @@ -39,8 +39,8 @@ /** * Reverse-complement bases and reverse quality scores along with known optional attributes that - * need the same treatment. This does not make a deep copy of the bases, qualities or attributes: - * for safe reverseComplement see {@link #reverseComplement(SAMRecord, boolean)}. + * need the same treatment. Changes made in-place, instead of making a copy of the bases, qualities, + * or attributes. If a copy is needed use {@link #reverseComplement(SAMRecord, boolean)}. * See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} * for the default set of tags that are handled. */ @@ -50,15 +50,15 @@ public static void reverseComplement(final SAMRecord rec) { /** * Reverse-complement bases and reverse quality scores along with known optional attributes that - * need the same treatment. Optionally makes a deep copy of the bases, qualities or attributes. - * See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} + * need the same treatment. Optionally makes a copy of the bases, qualities or attributes instead + * of altering them in-place. See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} * for the default set of tags that are handled. * * @param rec Record to reverse complement. - * @param unsafe Setting this to false will clone all attributes, bases and qualities before changing the values. + * @param inplace Setting this to false will clone all attributes, bases and qualities before changing the values. */ - public static void reverseComplement(final SAMRecord rec, boolean unsafe) { - reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, unsafe); + public static void reverseComplement(final SAMRecord rec, boolean inplace) { + reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, inplace); } /** @@ -66,11 +66,11 @@ public static void reverseComplement(final SAMRecord rec, boolean unsafe) { * non-null attributes specified by tagsToRevcomp and reverse and non-null attributes * specified by tagsToReverse. */ - public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse, boolean unsafe) { - final byte[] readBases = unsafe ? rec.getReadBases() : rec.getReadBases().clone(); + public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse, boolean inplace) { + final byte[] readBases = inplace ? rec.getReadBases() : rec.getReadBases().clone(); SequenceUtil.reverseComplement(readBases); rec.setReadBases(readBases); - final byte qualities[] = unsafe ? rec.getBaseQualities() : rec.getBaseQualities().clone(); + final byte qualities[] = inplace ? rec.getBaseQualities() : rec.getBaseQualities().clone(); reverseArray(qualities); rec.setBaseQualities(qualities); @@ -80,10 +80,11 @@ public static void reverseComplement(final SAMRecord rec, final Collection Date: Tue, 4 Oct 2016 17:49:47 -0400 Subject: [PATCH 4/6] Refactor reverseComplement to SAMRecord and make default copy attributes --- src/main/java/htsjdk/samtools/SAMRecord.java | 143 +++++++++++++++++++-- src/main/java/htsjdk/samtools/SAMRecordUtil.java | 97 +------------- .../java/htsjdk/samtools/SAMRecordUnitTest.java | 51 ++++++++ .../java/htsjdk/samtools/SAMRecordUtilTest.java | 61 --------- 4 files changed, 190 insertions(+), 162 deletions(-) delete mode 100644 src/test/java/htsjdk/samtools/SAMRecordUtilTest.java diff --git a/src/main/java/htsjdk/samtools/SAMRecord.java b/src/main/java/htsjdk/samtools/SAMRecord.java index 7b6b45e58..a24ea7ac1 100644 --- a/src/main/java/htsjdk/samtools/SAMRecord.java +++ b/src/main/java/htsjdk/samtools/SAMRecord.java @@ -26,17 +26,12 @@ import htsjdk.samtools.util.CoordMath; import htsjdk.samtools.util.Locatable; +import htsjdk.samtools.util.SequenceUtil; import htsjdk.samtools.util.StringUtil; import java.io.Serializable; import java.lang.reflect.Array; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; /** @@ -161,6 +156,16 @@ */ public static final int MAX_INSERT_SIZE = 1<<29; + /** + * Tags that are known to need the reverse complement if the read is reverse complemented. + */ + public static List TAGS_TO_REVERSE_COMPLEMENT = Arrays.asList(SAMTag.E2.name(), SAMTag.SQ.name()); + + /** + * Tags that are known to need the reverse if the read is reverse complemented. + */ + public static List TAGS_TO_REVERSE = Arrays.asList(SAMTag.OQ.name(), SAMTag.U2.name()); + private String mReadName = null; private byte[] mReadBases = NULL_SEQUENCE; private byte[] mBaseQualities = NULL_QUALS; @@ -2249,5 +2254,127 @@ public final Object removeTransientAttribute(final Object key) { if (this.transientAttributes != null) return this.transientAttributes.remove(key); else return null; } -} + /** + * 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 attributes. If in-place update is needed use {@link #reverseComplement(SAMRecord, boolean)}. + * See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} + * for the default set of tags that are handled. + */ + public static void reverseComplement(final SAMRecord rec) { + reverseComplement(rec, false); + } + + /** + * Reverse-complement bases and reverse quality scores along with known optional attributes that + * need the same treatment. Optionally makes a copy of the bases, qualities or attributes instead + * of altering them in-place. See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} + * for the default set of tags that are handled. + * + * @param rec Record to reverse complement. + * @param inplace Setting this to false will clone all attributes, bases and qualities before changing the values. + */ + public static void reverseComplement(final SAMRecord rec, boolean inplace) { + reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, inplace); + } + + /** + * Reverse complement bases and reverse quality scores. In addition reverse complement any + * non-null attributes specified by tagsToRevcomp and reverse and non-null attributes + * specified by tagsToReverse. + */ + public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse, boolean inplace) { + final byte[] readBases = inplace ? rec.getReadBases() : rec.getReadBases().clone(); + SequenceUtil.reverseComplement(readBases); + rec.setReadBases(readBases); + final byte qualities[] = inplace ? rec.getBaseQualities() : rec.getBaseQualities().clone(); + reverseArray(qualities); + rec.setBaseQualities(qualities); + + // Deal with tags that need to be reverse complemented + if (tagsToRevcomp != null) { + for (final String tag: tagsToRevcomp) { + Object value = rec.getAttribute(tag); + if (value != null) { + if (value instanceof byte[]) { + value = inplace ? value : ((byte[]) value).clone(); + SequenceUtil.reverseComplement((byte[]) value); + } + else if (value instanceof String) { + //SequenceUtil.reverseComplement is in-place for bytes but copies Strings since they are immutable. + value = SequenceUtil.reverseComplement((String) value); + } + else throw new UnsupportedOperationException("Don't know how to reverse complement: " + value); + rec.setAttribute(tag, value); + } + } + } + + // Deal with tags that needed to just be reversed + if (tagsToReverse != null) { + for (final String tag : tagsToReverse) { + Object value = rec.getAttribute(tag); + if (value != null) { + if (value instanceof String) { + value = StringUtil.reverseString((String) value); + } + else if (value.getClass().isArray()) { + if (value instanceof byte[]) { + value = inplace ? value : ((byte[]) value).clone(); + reverseArray((byte[]) value); + } + else if (value instanceof short[]) { + value = inplace ? value : ((short[]) value).clone(); + reverseArray((short[]) value); + } + else if (value instanceof int[]) { + value = inplace ? value : ((int[]) value).clone(); + reverseArray((int[]) value); + } + else if (value instanceof float[]) { + value = inplace ? value : ((float[]) value).clone(); + reverseArray((float[]) value); + } + else throw new UnsupportedOperationException("Reversing array attribute of type " + value.getClass().getComponentType() + " not supported."); + } + else throw new UnsupportedOperationException("Don't know how to reverse: " + value); + + rec.setAttribute(tag, value); + } + } + } + } + + private static void reverseArray(final byte[] array) { + for (int i=0, j=array.length-1; i TAGS_TO_REVERSE_COMPLEMENT = Arrays.asList(SAMTag.E2.name(), SAMTag.SQ.name()); public static List TAGS_TO_REVERSE = Arrays.asList(SAMTag.OQ.name(), SAMTag.U2.name()); @@ -45,7 +46,7 @@ * for the default set of tags that are handled. */ public static void reverseComplement(final SAMRecord rec) { - reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, true); + SAMRecord.reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, true); } /** @@ -58,7 +59,7 @@ public static void reverseComplement(final SAMRecord rec) { * @param inplace Setting this to false will clone all attributes, bases and qualities before changing the values. */ public static void reverseComplement(final SAMRecord rec, boolean inplace) { - reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, inplace); + SAMRecord.reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, inplace); } /** @@ -67,96 +68,6 @@ public static void reverseComplement(final SAMRecord rec, boolean inplace) { * specified by tagsToReverse. */ public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse, boolean inplace) { - final byte[] readBases = inplace ? rec.getReadBases() : rec.getReadBases().clone(); - SequenceUtil.reverseComplement(readBases); - rec.setReadBases(readBases); - final byte qualities[] = inplace ? rec.getBaseQualities() : rec.getBaseQualities().clone(); - reverseArray(qualities); - rec.setBaseQualities(qualities); - - // Deal with tags that need to be reverse complemented - if (tagsToRevcomp != null) { - for (final String tag: tagsToRevcomp) { - Object value = rec.getAttribute(tag); - if (value != null) { - if (value instanceof byte[]) { - value = inplace ? value : ((byte[]) value).clone(); - SequenceUtil.reverseComplement((byte[]) value); - } - else if (value instanceof String) { - //SequenceUtil.reverseComplement is in-place for bytes but copies Strings since they are immutable. - value = SequenceUtil.reverseComplement((String) value); - } - else throw new UnsupportedOperationException("Don't know how to reverse complement: " + value); - rec.setAttribute(tag, value); - } - } - } - - // Deal with tags that needed to just be reversed - if (tagsToReverse != null) { - for (final String tag : tagsToReverse) { - Object value = rec.getAttribute(tag); - if (value != null) { - if (value instanceof String) { - value = StringUtil.reverseString((String) value); - } - else if (value.getClass().isArray()) { - if (value instanceof byte[]) { - value = inplace ? value : ((byte[]) value).clone(); - reverseArray((byte[]) value); - } - else if (value instanceof short[]) { - value = inplace ? value : ((short[]) value).clone(); - reverseArray((short[]) value); - } - else if (value instanceof int[]) { - value = inplace ? value : ((int[]) value).clone(); - reverseArray((int[]) value); - } - else if (value instanceof float[]) { - value = inplace ? value : ((float[]) value).clone(); - reverseArray((float[]) value); - } - else throw new UnsupportedOperationException("Reversing array attribute of type " + value.getClass().getComponentType() + " not supported."); - } - else throw new UnsupportedOperationException("Don't know how to reverse: " + value); - - rec.setAttribute(tag, value); - } - } - } - } - - private static void reverseArray(final byte[] array) { - for (int i=0, j=array.length-1; i Date: Tue, 11 Oct 2016 09:24:47 -0400 Subject: [PATCH 5/6] Addressing new comments --- src/main/java/htsjdk/samtools/SAMRecord.java | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/main/java/htsjdk/samtools/SAMRecord.java b/src/main/java/htsjdk/samtools/SAMRecord.java index a24ea7ac1..3d36e9082 100644 --- a/src/main/java/htsjdk/samtools/SAMRecord.java +++ b/src/main/java/htsjdk/samtools/SAMRecord.java @@ -2258,7 +2258,8 @@ public final Object removeTransientAttribute(final Object key) { /** * 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 attributes. If in-place update is needed use {@link #reverseComplement(SAMRecord, boolean)}. + * and any attributes that will be altered. If in-place update is needed use + * {@link #reverseComplement(SAMRecord, boolean)}. * See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} * for the default set of tags that are handled. */ @@ -2300,12 +2301,12 @@ public static void reverseComplement(final SAMRecord rec, final Collection Date: Fri, 21 Oct 2016 10:42:47 -0400 Subject: [PATCH 6/6] latest comments --- src/main/java/htsjdk/samtools/SAMRecord.java | 29 ++++++++--------- src/main/java/htsjdk/samtools/SAMRecordUtil.java | 10 ++++-- .../java/htsjdk/samtools/SAMRecordUnitTest.java | 38 +++++++++++++++------- 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/main/java/htsjdk/samtools/SAMRecord.java b/src/main/java/htsjdk/samtools/SAMRecord.java index 3d36e9082..3bb11ec2b 100644 --- a/src/main/java/htsjdk/samtools/SAMRecord.java +++ b/src/main/java/htsjdk/samtools/SAMRecord.java @@ -2259,12 +2259,12 @@ public final Object removeTransientAttribute(final Object key) { * 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 - * {@link #reverseComplement(SAMRecord, boolean)}. + * {@link #reverseComplement(boolean)}. * See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} * for the default set of tags that are handled. */ - public static void reverseComplement(final SAMRecord rec) { - reverseComplement(rec, false); + public void reverseComplement() { + reverseComplement(false); } /** @@ -2273,11 +2273,10 @@ public static void reverseComplement(final SAMRecord rec) { * of altering them in-place. See {@link #TAGS_TO_REVERSE_COMPLEMENT} {@link #TAGS_TO_REVERSE} * for the default set of tags that are handled. * - * @param rec Record to reverse complement. * @param inplace Setting this to false will clone all attributes, bases and qualities before changing the values. */ - public static void reverseComplement(final SAMRecord rec, boolean inplace) { - reverseComplement(rec, TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, inplace); + public void reverseComplement(boolean inplace) { + reverseComplement(TAGS_TO_REVERSE_COMPLEMENT, TAGS_TO_REVERSE, inplace); } /** @@ -2285,18 +2284,18 @@ public static void reverseComplement(final SAMRecord rec, boolean inplace) { * non-null attributes specified by tagsToRevcomp and reverse and non-null attributes * specified by tagsToReverse. */ - public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse, boolean inplace) { - final byte[] readBases = inplace ? rec.getReadBases() : rec.getReadBases().clone(); + public void reverseComplement(final Collection tagsToRevcomp, final Collection tagsToReverse, boolean inplace) { + final byte[] readBases = inplace ? getReadBases() : getReadBases().clone(); SequenceUtil.reverseComplement(readBases); - rec.setReadBases(readBases); - final byte qualities[] = inplace ? rec.getBaseQualities() : rec.getBaseQualities().clone(); + setReadBases(readBases); + final byte qualities[] = inplace ? getBaseQualities() : getBaseQualities().clone(); reverseArray(qualities); - rec.setBaseQualities(qualities); + setBaseQualities(qualities); // Deal with tags that need to be reverse complemented if (tagsToRevcomp != null) { for (final String tag: tagsToRevcomp) { - Object value = rec.getAttribute(tag); + Object value = getAttribute(tag); if (value != null) { if (value instanceof byte[]) { value = inplace ? value : ((byte[]) value).clone(); @@ -2307,7 +2306,7 @@ public static void reverseComplement(final SAMRecord rec, final Collection tagsToRevcomp, final Collection tagsToReverse, boolean inplace) { - SAMRecord.reverseComplement(rec, tagsToRevcomp, tagsToReverse, inplace); + rec.reverseComplement(tagsToRevcomp, tagsToReverse, inplace); } } diff --git a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java index 140fede2e..951ecee78 100644 --- a/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java +++ b/src/test/java/htsjdk/samtools/SAMRecordUnitTest.java @@ -973,10 +973,11 @@ public void testResolveNameNullHeader() { SAMRecord.resolveNameFromIndex(1, null); } - @Test public void testReverseComplement() { + @Test + public void testReverseComplement() { final SAMRecord rec = createTestSamRec(); - SAMRecord.reverseComplement(rec, Arrays.asList("Y1"), Arrays.asList("X1", "X2", "X3", "X4", "X5"), true); + rec.reverseComplement(Arrays.asList("Y1"), Arrays.asList("X1", "X2", "X3", "X4", "X5"), false); Assert.assertEquals(rec.getReadString(), "GTGTGTGTGT"); Assert.assertEquals(rec.getBaseQualityString(), "IIIIIHHHHH"); Assert.assertEquals(rec.getByteArrayAttribute("X1"), new byte[] {5,4,3,2,1}); @@ -986,19 +987,33 @@ public void testResolveNameNullHeader() { Assert.assertEquals(rec.getStringAttribute("Y1"), "GTTTTCTTTT"); } - @Test public void testSafeReverseComplement() throws CloneNotSupportedException { + /** + * Note that since strings are immutable the Y1 attribute, which is a String, is not reversed in the original even + * if an in-place reverse complement occurred. The bases and qualities are byte[] so they are reversed if in-place + * is true. + */ + @DataProvider + public Object [][] reverseComplementData() { + return new Object[][]{ + {false, "ACACACACAC", "HHHHHIIIII", "AAAAGAAAAC", new byte[] {1,2,3,4,5}, new short[] {1,2,3,4,5}, new int[] {1,2,3,4,5}, new float[] {1,2,3,4,5}}, + {true, "GTGTGTGTGT", "IIIIIHHHHH", "AAAAGAAAAC", new byte[] {5,4,3,2,1}, new short[] {5,4,3,2,1}, new int[] {5,4,3,2,1}, new float[] {5,4,3,2,1}}, + }; + } + + @Test(dataProvider = "reverseComplementData") + public void testSafeReverseComplement(boolean inplace, String bases, String quals, String y1, byte[] x1, short[] x2, int[] x3, float[] x4) throws CloneNotSupportedException { final SAMRecord original = createTestSamRec(); final SAMRecord cloneOfOriginal = (SAMRecord) original.clone(); //Runs a copy (rather than in-place) reverseComplement - SAMRecord.reverseComplement(cloneOfOriginal, Arrays.asList("Y1"), Arrays.asList("X1", "X2", "X3", "X4", "X5"), false); + cloneOfOriginal.reverseComplement(Arrays.asList("Y1"), Arrays.asList("X1", "X2", "X3", "X4", "X5"), inplace); - Assert.assertEquals(original.getReadString(), "ACACACACAC"); - Assert.assertEquals(original.getBaseQualityString(), "HHHHHIIIII"); - Assert.assertEquals(original.getByteArrayAttribute("X1"), new byte[] {1,2,3,4,5}); - Assert.assertEquals(original.getSignedShortArrayAttribute("X2"), new short[] {1,2,3,4,5}); - Assert.assertEquals(original.getSignedIntArrayAttribute("X3"), new int[] {1,2,3,4,5}); - Assert.assertEquals(original.getFloatArrayAttribute("X4"), new float[] {1.0f,2.0f,3.0f,4.0f,5.0f}); - Assert.assertEquals(original.getStringAttribute("Y1"), "AAAAGAAAAC"); + Assert.assertEquals(original.getReadString(), bases); + Assert.assertEquals(original.getBaseQualityString(), quals); + Assert.assertEquals(original.getByteArrayAttribute("X1"), x1); + Assert.assertEquals(original.getSignedShortArrayAttribute("X2"), x2); + Assert.assertEquals(original.getSignedIntArrayAttribute("X3"), x3); + Assert.assertEquals(original.getFloatArrayAttribute("X4"), x4); + Assert.assertEquals(original.getStringAttribute("Y1"), y1); Assert.assertEquals(cloneOfOriginal.getReadString(), "GTGTGTGTGT"); Assert.assertEquals(cloneOfOriginal.getBaseQualityString(), "IIIIIHHHHH"); @@ -1023,5 +1038,4 @@ public SAMRecord createTestSamRec() { return(rec); } - }