From 2c501364b80697c5859c20fd8dc30b3421272bf8 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 21 Nov 2011 14:34:31 -0500 Subject: [PATCH] GenotypesContext no longer have immutability in constructor -- additional bug fixes throughout VariantContext and GenotypesContext objects --- .../variantcontext/GenotypesContext.java | 57 +++++++++++++------ .../variantcontext/LazyGenotypesContext.java | 4 +- .../VariantAnnotatorIntegrationTest.java | 2 +- .../GenotypesContextUnitTest.java | 12 +++- .../VariantContextUnitTest.java | 3 +- 5 files changed, 55 insertions(+), 23 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/variantcontext/GenotypesContext.java b/public/java/src/org/broadinstitute/sting/utils/variantcontext/GenotypesContext.java index 25b277298..8d28ba18c 100644 --- a/public/java/src/org/broadinstitute/sting/utils/variantcontext/GenotypesContext.java +++ b/public/java/src/org/broadinstitute/sting/utils/variantcontext/GenotypesContext.java @@ -38,7 +38,7 @@ public class GenotypesContext implements List { * static constant value for an empty GenotypesContext. Useful since so many VariantContexts have no genotypes */ public final static GenotypesContext NO_GENOTYPES = - new GenotypesContext(new ArrayList(0), new HashMap(0), Collections.emptyList(), true); + new GenotypesContext(new ArrayList(0), new HashMap(0), Collections.emptyList()).immutable(); /** *sampleNamesInOrder a list of sample names, one for each genotype in genotypes, sorted in alphabetical order @@ -77,24 +77,23 @@ public class GenotypesContext implements List { * Create an empty GenotypeContext */ protected GenotypesContext() { - this(10, false); + this(10); } /** * Create an empty GenotypeContext, with initial capacity for n elements */ @Requires("n >= 0") - protected GenotypesContext(final int n, final boolean immutable) { - this(new ArrayList(n), immutable); + protected GenotypesContext(final int n) { + this(new ArrayList(n)); } /** * Create an GenotypeContext containing genotypes */ - @Requires("genotypes != null") - protected GenotypesContext(final ArrayList genotypes, final boolean immutable) { + @Requires({"genotypes != null", "noDups(genotypes)"}) + protected GenotypesContext(final ArrayList genotypes) { this.notToBeDirectlyAccessedGenotypes = genotypes; - this.immutable = immutable; this.sampleNameToOffset = null; this.cacheIsInvalid = true; } @@ -109,19 +108,16 @@ public class GenotypesContext implements List { * genotype in the vector of genotypes * @param sampleNamesInOrder a list of sample names, one for each genotype in genotypes, sorted in alphabetical * order. - * @param immutable */ - @Requires({"genotypes != null", + @Requires({"genotypes != null", "noDups(genotypes)", "sampleNameToOffset != null", "sampleNamesInOrder != null", "genotypes.size() == sampleNameToOffset.size()", "genotypes.size() == sampleNamesInOrder.size()"}) protected GenotypesContext(final ArrayList genotypes, final Map sampleNameToOffset, - final List sampleNamesInOrder, - final boolean immutable) { + final List sampleNamesInOrder) { this.notToBeDirectlyAccessedGenotypes = genotypes; - this.immutable = immutable; this.sampleNameToOffset = sampleNameToOffset; this.sampleNamesInOrder = sampleNamesInOrder; this.cacheIsInvalid = false; @@ -149,7 +145,7 @@ public class GenotypesContext implements List { @Requires("nGenotypes >= 0") @Ensures({"result != null"}) public static final GenotypesContext create(final int nGenotypes) { - return new GenotypesContext(nGenotypes, false); + return new GenotypesContext(nGenotypes); } /** @@ -173,7 +169,7 @@ public class GenotypesContext implements List { public static final GenotypesContext create(final ArrayList genotypes, final Map sampleNameToOffset, final List sampleNamesInOrder) { - return new GenotypesContext(genotypes, sampleNameToOffset, sampleNamesInOrder, false); + return new GenotypesContext(genotypes, sampleNameToOffset, sampleNamesInOrder); } /** @@ -185,7 +181,7 @@ public class GenotypesContext implements List { @Requires({"genotypes != null"}) @Ensures({"result != null"}) public static final GenotypesContext create(final ArrayList genotypes) { - return genotypes == null ? NO_GENOTYPES : new GenotypesContext(genotypes, false); + return genotypes == null ? NO_GENOTYPES : new GenotypesContext(genotypes); } /** @@ -197,7 +193,7 @@ public class GenotypesContext implements List { @Requires({"genotypes != null"}) @Ensures({"result != null"}) public static final GenotypesContext create(final Genotype... genotypes) { - return new GenotypesContext(new ArrayList(Arrays.asList(genotypes)), false); + return create(new ArrayList(Arrays.asList(genotypes))); } /** @@ -306,14 +302,16 @@ public class GenotypesContext implements List { } @Override - @Requires("genotype != null") + @Requires({"genotype != null", "get(genotype.getSampleName()) == null"}) + @Ensures("noDups(getGenotypes())") public boolean add(final Genotype genotype) { checkImmutability(); invalidateCaches(); return getGenotypes().add(genotype); } - @Requires("genotype != null") + @Requires({"genotype != null", "! containsAny(Arrays.asList(genotype))"}) + @Ensures("noDups(getGenotypes())") public boolean add(final Genotype ... genotype) { checkImmutability(); invalidateCaches(); @@ -321,11 +319,15 @@ public class GenotypesContext implements List { } @Override + @Requires("! contains(genotype)") + @Ensures("noDups(getGenotypes())") public void add(final int i, final Genotype genotype) { throw new UnsupportedOperationException(); } @Override + @Requires("! containsAny(genotypes)") + @Ensures("noDups(getGenotypes())") public boolean addAll(final Collection genotypes) { checkImmutability(); invalidateCaches(); @@ -347,6 +349,13 @@ public class GenotypesContext implements List { return getGenotypes().containsAll(objects); } + private boolean containsAny(final Collection genotypes) { + for ( final Genotype g : genotypes ) { + if ( contains(g) ) return true; + } + return false; + } + @Override public Genotype get(final int i) { return getGenotypes().get(i); @@ -421,6 +430,7 @@ public class GenotypesContext implements List { } @Override + @Ensures("noDups(getGenotypes())") public Genotype set(final int i, final Genotype genotype) { checkImmutability(); invalidateCaches(); @@ -604,6 +614,17 @@ public class GenotypesContext implements List { } } + protected final static boolean noDups(Collection genotypes) { + Set names = new HashSet(genotypes.size()); + for ( final Genotype g : genotypes ) { + if ( names.contains(g.getSampleName()) ) + return false; + names.add(g.getSampleName()); + } + + return true; + } + protected final static boolean sameSamples(List genotypes, Collection sampleNamesInOrder) { Set names = new HashSet(sampleNamesInOrder); if ( names.size() != sampleNamesInOrder.size() ) diff --git a/public/java/src/org/broadinstitute/sting/utils/variantcontext/LazyGenotypesContext.java b/public/java/src/org/broadinstitute/sting/utils/variantcontext/LazyGenotypesContext.java index b3a24aef5..5fbaadfab 100644 --- a/public/java/src/org/broadinstitute/sting/utils/variantcontext/LazyGenotypesContext.java +++ b/public/java/src/org/broadinstitute/sting/utils/variantcontext/LazyGenotypesContext.java @@ -73,7 +73,7 @@ public class LazyGenotypesContext extends GenotypesContext { /** * Returns the data used in the full GenotypesContext constructor * - * {@link GenotypesContext#GenotypesContext(java.util.ArrayList, java.util.Map, java.util.List, boolean)} + * {@link GenotypesContext#GenotypesContext(java.util.ArrayList, java.util.Map, java.util.List)} */ public static class LazyData { final ArrayList genotypes; @@ -102,7 +102,7 @@ public class LazyGenotypesContext extends GenotypesContext { */ @Requires({"parser != null", "unparsedGenotypeData != null", "nUnparsedGenotypes >= 0"}) public LazyGenotypesContext(final LazyParser parser, final Object unparsedGenotypeData, final int nUnparsedGenotypes) { - super(EMPTY, false); + super(EMPTY); this.parser = parser; this.unparsedGenotypeData = unparsedGenotypeData; this.nUnparsedGenotypes = nUnparsedGenotypes; diff --git a/public/java/test/org/broadinstitute/sting/gatk/walkers/annotator/VariantAnnotatorIntegrationTest.java b/public/java/test/org/broadinstitute/sting/gatk/walkers/annotator/VariantAnnotatorIntegrationTest.java index 75c27e429..1824789a9 100755 --- a/public/java/test/org/broadinstitute/sting/gatk/walkers/annotator/VariantAnnotatorIntegrationTest.java +++ b/public/java/test/org/broadinstitute/sting/gatk/walkers/annotator/VariantAnnotatorIntegrationTest.java @@ -58,7 +58,7 @@ public class VariantAnnotatorIntegrationTest extends WalkerTest { // they don't get reordered. It's a good test of the genotype ordering system. WalkerTestSpec spec = new WalkerTestSpec( baseTestString() + " --variant:VCF3 " + validationDataLocation + "vcfexample3empty.vcf -I " + validationDataLocation + "NA12878.1kg.p2.chr1_10mb_11_mb.SLX.bam -L 1:10,000,000-10,050,000", 1, - Arrays.asList("0cc0ec59f0328792e6413b6ff3f71780")); + Arrays.asList("f2ddfa8105c290b1f34b7a261a02a1ac")); executeTest("test file doesn't have annotations, not asking for annotations, #2", spec); } diff --git a/public/java/test/org/broadinstitute/sting/utils/variantcontext/GenotypesContextUnitTest.java b/public/java/test/org/broadinstitute/sting/utils/variantcontext/GenotypesContextUnitTest.java index c12fbac9c..afb9336a0 100644 --- a/public/java/test/org/broadinstitute/sting/utils/variantcontext/GenotypesContextUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/utils/variantcontext/GenotypesContextUnitTest.java @@ -77,6 +77,11 @@ public class GenotypesContextUnitTest extends BaseTest { public GenotypesContext make(final List initialSamples) { return GenotypesContext.copy(initialSamples); } + + @Override + public String toString() { + return "GenotypesContext"; + } }; private final class lazyMaker implements LazyGenotypesContext.LazyParser, ContextMaker { @@ -91,6 +96,11 @@ public class GenotypesContextUnitTest extends BaseTest { public GenotypesContext make(final List initialSamples) { return new LazyGenotypesContext(this, initialSamples, initialSamples.size()); } + + @Override + public String toString() { + return "LazyGenotypesContext"; + } } private Collection allMakers = Arrays.asList(baseMaker, new lazyMaker()); @@ -100,7 +110,7 @@ public class GenotypesContextUnitTest extends BaseTest { final List initialSamples; private GenotypesContextProvider(ContextMaker maker, List initialSamples) { - super(GenotypesContextProvider.class); + super(GenotypesContextProvider.class, String.format("%s with %d samples", maker.toString(), initialSamples.size())); this.maker = maker; this.initialSamples = initialSamples; } diff --git a/public/java/test/org/broadinstitute/sting/utils/variantcontext/VariantContextUnitTest.java b/public/java/test/org/broadinstitute/sting/utils/variantcontext/VariantContextUnitTest.java index a7eac7ab9..fca7440e4 100755 --- a/public/java/test/org/broadinstitute/sting/utils/variantcontext/VariantContextUnitTest.java +++ b/public/java/test/org/broadinstitute/sting/utils/variantcontext/VariantContextUnitTest.java @@ -834,7 +834,8 @@ public class VariantContextUnitTest extends BaseTest { for ( int j = 0; j < i; j++ ) { nSamples++; Genotype g = allGenotypes.get(j % allGenotypes.size()); - gc.add(g); + final String name = String.format("%s_%d%d", g.getSampleName(), i, j); + gc.add(new Genotype(name, g.getAlleles())); switch ( g.getType() ) { case NO_CALL: nNoCall++; nNoCallAlleles++; break; case HOM_REF: nA += 2; nHomRef++; break;