diff --git a/.gitignore b/.gitignore index 694f85de06..f10fff99c0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,5 @@ htsjdk.iws .command_tmp -.DS_Store atlassian-ide-plugin.xml /htsjdk.version.properties /test-output/ diff --git a/src/main/java/htsjdk/variant/variantcontext/VariantContextComparator.java b/src/main/java/htsjdk/variant/variantcontext/VariantContextComparator.java index bcccbeab4e..d4e288f978 100644 --- a/src/main/java/htsjdk/variant/variantcontext/VariantContextComparator.java +++ b/src/main/java/htsjdk/variant/variantcontext/VariantContextComparator.java @@ -83,20 +83,11 @@ public int compare(final VariantContext firstVariantContext, final VariantContex // Will throw NullPointerException -- happily -- if either of the chromosomes/contigs aren't // present. This error checking should already have been done in the constructor but it's left // in as defence anyway. - int contigCompare = this.contigIndexLookup.get(firstVariantContext.getContig()).compareTo(this.contigIndexLookup.get(secondVariantContext.getContig())); - contigCompare = contigCompare == 0 ? firstVariantContext.getStart() - secondVariantContext.getStart() : contigCompare; - if (contigCompare == 0) { - // Compare variants that have the same genomic span (chr:start-end) lexicographically by all alleles (ref and alts). - for (int i = 0; i < firstVariantContext.getAlleles().size(); i++) { - // If all previous alleles are identical and the first variant has additional alleles, make the first variant greater. - if (i >= secondVariantContext.getAlleles().size()) { return 1; } - contigCompare = firstVariantContext.getAlleles().get(i).compareTo(secondVariantContext.getAlleles().get(i)); - if (contigCompare != 0) return contigCompare; - } - } - // If all previous alleles are identical and the second variant has additional alleles, make the second variant greater. - if (firstVariantContext.getAlleles().size() < secondVariantContext.getAlleles().size()) { return -1; } - return contigCompare; + final int contigCompare = + this.contigIndexLookup.get(firstVariantContext.getContig()) - this.contigIndexLookup.get(secondVariantContext.getContig()); + return contigCompare != 0 + ? contigCompare + : firstVariantContext.getStart() - secondVariantContext.getStart(); } /** diff --git a/src/test/java/htsjdk/variant/variantcontext/VariantContextComparatorUnitTest.java b/src/test/java/htsjdk/variant/variantcontext/VariantContextComparatorUnitTest.java deleted file mode 100644 index 578f673ae4..0000000000 --- a/src/test/java/htsjdk/variant/variantcontext/VariantContextComparatorUnitTest.java +++ /dev/null @@ -1,75 +0,0 @@ -package htsjdk.variant.variantcontext; - -import htsjdk.HtsjdkTest; -import org.testng.Assert; -import org.testng.annotations.BeforeSuite; -import org.testng.annotations.Test; - -import java.util.Arrays; -import java.util.Collections; - -/** - * Unit tests for VariantContextComparator. - */ -public class VariantContextComparatorUnitTest extends HtsjdkTest { - private Allele refA, altG, altT; - - @BeforeSuite - public void before() { - refA = Allele.create("A", true); - altG = Allele.create("G", false); - altT = Allele.create("T", false); - } - - @Test - public void testVariantContextsWithSameSiteSortLexicographicallyByAlleleIdentical() { - final String contig = "chr1"; - final VariantContextComparator comparator = new VariantContextComparator(Collections.singletonList(contig)); - final VariantContextBuilder builder = new VariantContextBuilder("test", contig, 1, 1, Collections.emptyList()); - - final VariantContext variant1 = builder.alleles(Arrays.asList(refA, altG)).make(); - final VariantContext variant2 = builder.alleles(Arrays.asList(refA, altG)).make(); - - final int compare = comparator.compare(variant1, variant2); - Assert.assertEquals(compare, 0); // TODO: What other criteria might we sort by to break this tie? - } - - @Test - public void testVariantContextsWithSameSiteSortLexicographicallyByAllele() { - final String contig = "chr1"; - final VariantContextComparator comparator = new VariantContextComparator(Collections.singletonList(contig)); - final VariantContextBuilder builder = new VariantContextBuilder("test", contig, 1, 1, Collections.emptyList()); - - final VariantContext variant1 = builder.alleles(Arrays.asList(refA, altG)).make(); - final VariantContext variant2 = builder.alleles(Arrays.asList(refA, altT)).make(); - - final int compare = comparator.compare(variant1, variant2); - Assert.assertEquals(compare, -1); - } - - @Test - public void testVariantContextsWithSameSiteSortLexicographicallyByAlleleThenExtraAllelesForFirstVariant() { - final String contig = "chr1"; - final VariantContextComparator comparator = new VariantContextComparator(Collections.singletonList(contig)); - final VariantContextBuilder builder = new VariantContextBuilder("test", contig, 1, 1, Collections.emptyList()); - - final VariantContext variant1 = builder.alleles(Arrays.asList(refA, altG, altT)).make(); - final VariantContext variant2 = builder.alleles(Arrays.asList(refA, altG)).make(); - - final int compare = comparator.compare(variant1, variant2); - Assert.assertEquals(compare, 1); - } - - @Test - public void testVariantContextsWithSameSiteSortLexicographicallyByAlleleThenExtraAllelesForSecondVariant() { - final String contig = "chr1"; - final VariantContextComparator comparator = new VariantContextComparator(Collections.singletonList(contig)); - final VariantContextBuilder builder = new VariantContextBuilder("test", contig, 1, 1, Collections.emptyList()); - - final VariantContext variant1 = builder.alleles(Arrays.asList(refA, altG)).make(); - final VariantContext variant2 = builder.alleles(Arrays.asList(refA, altG, altT)).make(); - - final int compare = comparator.compare(variant1, variant2); - Assert.assertEquals(compare, -1); - } -}