Skip to content

Commit

Permalink
Moderate refactoring of VariantContext type determination
Browse files Browse the repository at this point in the history
- This was necessary because the type caching needs to distinguish between ignoreNonRef being true or false
- Changed return type of `determineType` and `determinePolymorphicType` from `void` to `VariantContext.Type`, otherwise multiple code branches would be necessary depending on which caching variable to set
- Added unit test to catch if the cache separation works
  • Loading branch information
michaelgatzen committed Apr 12, 2021
1 parent 2d066da commit 13a84bd
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 22 deletions.
52 changes: 30 additions & 22 deletions src/main/java/htsjdk/variant/variantcontext/VariantContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ public class VariantContext implements Feature, Serializable {
/** The type (cached for performance reasons) of this context */
protected Type type = null;

/** The type of this context, cached separately if ignoreNonRef is true */
protected Type typeIgnoringNonRef = null;

/** A set of the alleles segregating in this context */
protected final List<Allele> alleles;

Expand Down Expand Up @@ -676,10 +679,18 @@ public Type getType() {
* @return the type of this VariantContext
**/
public Type getType(final boolean ignoreNonRef) {
if ( type == null )
determineType(ignoreNonRef);

return type;
// Make sure we use the correct cached result
if (ignoreNonRef) {
if (typeIgnoringNonRef == null) {
typeIgnoringNonRef = determineType(ignoreNonRef);
}
return typeIgnoringNonRef;
} else {
if (type == null) {
type = determineType(ignoreNonRef);
}
return type;
}
}

/**
Expand Down Expand Up @@ -1426,24 +1437,21 @@ private void validateStop() {
//
// ---------------------------------------------------------------------------------------------------------

private void determineType(final boolean ignoreNonRef) {
if ( type == null ) {
switch ( getNAlleles() ) {
case 0:
throw new IllegalStateException("Unexpected error: requested type of VariantContext with no alleles!" + this);
case 1:
// note that this doesn't require a reference allele. You can be monomorphic independent of having a
// reference allele
type = Type.NO_VARIATION;
break;
default:
determinePolymorphicType(ignoreNonRef);
}
private Type determineType(final boolean ignoreNonRef) {
switch ( getNAlleles() ) {
case 0:
throw new IllegalStateException("Unexpected error: requested type of VariantContext with no alleles!" + this);
case 1:
// note that this doesn't require a reference allele. You can be monomorphic independent of having a
// reference allele
return Type.NO_VARIATION;
default:
return determinePolymorphicType(ignoreNonRef);
}
}

private void determinePolymorphicType(final boolean ignoreNonRef) {
type = null;
private Type determinePolymorphicType(final boolean ignoreNonRef) {
Type type = null;
boolean nonRefAlleleFound = false;

// do a pairwise comparison of all alleles against the reference allele
Expand All @@ -1466,15 +1474,15 @@ private void determinePolymorphicType(final boolean ignoreNonRef) {
}
// if the type of this allele is different from that of a previous one, assign it the MIXED type and quit
else if ( biallelicType != type ) {
type = Type.MIXED;
return;
return Type.MIXED;
}
}
// If all alt alleles are NON_REF alleles and ignoreNonRef is true, type will still be null. Therefore, if we
// have only seen NON_REFs, choose SYMBOLIC
if (type == null && nonRefAlleleFound) {
type = Type.SYMBOLIC;
return Type.SYMBOLIC;
}
return type;
}

private static Type typeOfBiallelicVariant(Allele ref, Allele allele) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,13 @@ public void testDetermineTypes() {
alleles = Arrays.asList(Aref, C, symbolic, nonRef);
vc = basicBuilder.alleles(alleles).stop(snpLocStop).make();
Assert.assertEquals(vc.getType(true), VariantContext.Type.MIXED);

// Assure that the caching of the variant type is not persistent between ignoreNonRef being true and false
alleles = Arrays.asList(Aref, C, nonRef);
vc = basicBuilder.alleles(alleles).stop(snpLocStop).make();
Assert.assertEquals(vc.getType(), VariantContext.Type.MIXED);
Assert.assertEquals(vc.getType(true), VariantContext.Type.SNP);
Assert.assertEquals(vc.getType(), VariantContext.Type.MIXED);
}

@Test
Expand Down

0 comments on commit 13a84bd

Please sign in to comment.