VariantContextBuilder does not perform a deep copy

[d-cameron]

When using VariantContextBuilder to create a new variant based on an existing variant, the underlying objects and collections are not copied nor are they wrapped with copy-on-write semantics. Making changes to a copied variant change the original variant as well.

Failing unit test for VariantContextUnitTest.java

@Test
    public void testFilterUnaffectedByClonedVariants() {
        VariantContext vc1 = new VariantContextBuilder("source", "contig", 1, 1, Arrays.asList(Tref, A)).filter("TEST").make();
        VariantContext vc2 = new VariantContextBuilder(vc1).filter("TEST2").make();
        Assert.assertFalse(vc1.getFilters().equals(vc2.getFilters()));
    }

[yfarjoun]

attributes are protected against this, I guess filters need the same
treatment.

On Tue, May 19, 2015 at 3:13 AM, Daniel Cameron notifications@github.com
wrote:

When using VariantContextBuilder to create a new variant based on an
existing variant, the underlying objects and collections are not copied nor
are they wrapped with copy-on-write semantics. Making changes to a copied
variant change the original variant as well.

Failing unit test for VariantContextUnitTest.java

@test
public void testFilterUnaffectedByClonedVariants() {
VariantContext vc1 = new VariantContextBuilder("source", "contig", 1, 1, Arrays.asList(Tref, A)).filter("TEST").make();
VariantContext vc2 = new VariantContextBuilder(vc1).filter("TEST2").make();
Assert.assertFalse(vc1.getFilters().equals(vc2.getFilters()));
}

—
Reply to this email directly or view it on GitHub
#255.

[d-cameron]

Yes, attributesCanBeModified does look pretty obvious when reviewing the code again. Depending on when GenotypesContext is/is not immutable, genotypes might also be an issue.

[yfarjoun]

also include a test without calling make():

@Test
    public void testFilterUnaffectedByClonedVariants() {
        VariantContext vc1 = new VariantContextBuilder("source", "contig", 1, 1, Arrays.asList(Tref, A)).filter("TEST").make();
        VariantContext vc2 = new VariantContextBuilder(vc1).filter("TEST2");
        Assert.assertFalse(vc1.getFilters().equals(vc2.getFilters()));
    }

[yfarjoun]

this is actually worse. I can get it to fail on filters (as reported) attributes, genotypes and alleles....

[yfarjoun]

submitting a PR with tests for comments.