Change length calculation (or checking) might need a revision

[ielis]

@julesjacobsen - let's create a deletion chr1:1AA>A that was trimmed in a way such that the common base was removed:

@Test
public void delTrimmed() {
    Variant del = DefaultVariant.oneBased(chr1, 2, "A", ""); // throws an IllegalArgumentException

    assertThat(del.startPosition(), equalTo(Position.of(2)));
    assertThat(del.endPosition(), equalTo(Position.of(2)));
    assertThat(del.variantType(), equalTo(VariantType.DEL));
    assertThat(del.refLength(), equalTo(1));
    assertThat(del.length(), equalTo(0));
    assertThat(del.changeLength(), equalTo(-1));
}

When placed into DefaultVariantTest, the test above throws an exception in the constructor.

I think that it is a valid use case. We might need to revise the change length calculation/checking.

[julesjacobsen]

Hmm, indeed.

I think we just need to relax the BaseVariant.checkChangeLength for deletions to be in line with the other types. i.e.

private int checkChangeLength(int changeLength, VariantType variantType) {
// change this
    int start = startWithCoordinateSystem(CoordinateSystem.oneBased());
    int end = endWithCoordinateSystem(CoordinateSystem.oneBased());
    if (variantType.baseType() == VariantType.DEL && start - end != changeLength) {
            throw new IllegalArgumentException("Illegal DEL changeLength:" + changeLength + ". Does not match expected " + (start - end) + " given coordinates " + coordinates());
// to this
    if (variantType.baseType() == VariantType.DEL && (changeLength >= 0)) {

then these tests pass:

        @Test
        public void delZeroBasedTrimmedToEmpty() {
            Variant del = DefaultVariant.zeroBased(chr1, 1, "G", "");

            assertThat(del.startPosition(), equalTo(Position.of(1)));
            assertThat(del.endPosition(), equalTo(Position.of(2)));
            assertThat(del.variantType(), equalTo(VariantType.DEL));
            assertThat(del.refLength(), equalTo(1));
            assertThat(del.length(), equalTo(1));
            assertThat(del.changeLength(), equalTo(-1));
        }

        @Test
        public void delOneBasedTrimmedToEmpty() {
            Variant del = DefaultVariant.oneBased(chr1, 2, "A", "");

            assertThat(del.startPosition(), equalTo(Position.of(2)));
            assertThat(del.endPosition(), equalTo(Position.of(2)));
            assertThat(del.variantType(), equalTo(VariantType.DEL));
            assertThat(del.refLength(), equalTo(1));
            assertThat(del.length(), equalTo(1)); // note this should be 1, not 0
            assertThat(del.changeLength(), equalTo(-1));
        }

[ielis]

I think this works now