Merge pull request #1014 from broadinstitute/gg_fix_combinevariants_del_allele_1000

Added else clause to handle symbolic alleles

protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/variantutils/CombineVariantsIntegrationTest.java

@@ -223,4 +223,18 @@ public class CombineVariantsIntegrationTest extends WalkerTest {
             executeTest("combiningGVCFsFails", spec);
         } catch (Exception e) { } // do nothing
     }
+
+    @Test
+    public void combineSymbolicVariants() {
+        // Just checking that this does not fail, hence no output files and MD5
+        WalkerTestSpec spec = new WalkerTestSpec(
+                "-T CombineVariants --no_cmdline_in_header -o %s "
+                        + " -R " + hg19RefereneWithChrPrefixInChromosomeNames
+                        + " -V " + privateTestDir + "WES-chr1.DEL.vcf"
+                        + " -V " + privateTestDir + "WGS-chr1.DEL.vcf"
+                        + " -genotypeMergeOptions UNIQUIFY",
+                0,
+                Arrays.asList(""));
+        executeTest("combineSymbolicVariants: ", spec);
+    }
 }

public/gatk-utils/src/main/java/org/broadinstitute/gatk/utils/variant/GATKVariantContextUtils.java

@@ -393,7 +393,7 @@ public class GATKVariantContextUtils {
             if (repetitionCount[0] == 0 || repetitionCount[1] == 0)
                 return null;
 
-            if (lengths.size() == 0) {
+            if (lengths.isEmpty()) {
                 lengths.add(repetitionCount[0]); // add ref allele length only once
             }
             lengths.add(repetitionCount[1]);  // add this alt allele's length
@@ -947,7 +947,7 @@ public class GATKVariantContextUtils {
                                              final String setKey,
                                              final boolean filteredAreUncalled,
                                              final boolean mergeInfoWithMaxAC ) {
-        if ( unsortedVCs == null || unsortedVCs.size() == 0 )
+        if ( unsortedVCs == null || unsortedVCs.isEmpty() )
             return null;
 
         if (priorityListOfVCs != null && originalNumOfVCs != priorityListOfVCs.size())
@@ -965,7 +965,7 @@ public class GATKVariantContextUtils {
                 VCs.add(vc);
         }
 
-        if ( VCs.size() == 0 ) // everything is filtered out and we're filteredAreUncalled
+        if ( VCs.isEmpty() ) // everything is filtered out and we're filteredAreUncalled
             return null;
 
         // establish the baseline info from the first VC
@@ -1289,7 +1289,7 @@ public class GATKVariantContextUtils {
      * @param currentAlleles     the list of alleles already created
      * @return a non-null mapping of original alleles to new (extended) ones
      */
-    private static Map<Allele, Allele> createAlleleMapping(final Allele refAllele,
+    protected static Map<Allele, Allele> createAlleleMapping(final Allele refAllele,
                                                            final VariantContext oneVC,
                                                            final Collection<Allele> currentAlleles) {
         final Allele myRef = oneVC.getReference();
@@ -1305,6 +1305,8 @@ public class GATKVariantContextUtils {
                     if ( extended.equals(b) )
                         extended = b;
                 map.put(a, extended);
+            } else if ( a.isSymbolic() ) {
+                map.put(a, a);
             }
         }
 
@@ -1696,7 +1698,7 @@ public class GATKVariantContextUtils {
                         // otherVC has a type different than vc and its alleles are a subset of vc: remove otherVC from its list and add it to vc's type list
                         vcList.remove(k);
                         // avoid having empty lists
-                        if (vcList.size() == 0)
+                        if (vcList.isEmpty())
                             mappedVCs.remove(type);
                         if ( !mappedVCs.containsKey(vcType) )
                             mappedVCs.put(vcType, new ArrayList<VariantContext>());

public/gatk-utils/src/test/java/org/broadinstitute/gatk/utils/variant/GATKVariantContextUtilsUnitTest.java

@@ -46,6 +46,7 @@ public class GATKVariantContextUtilsUnitTest extends BaseTest {
     Allele ATref;
     Allele Anoref;
     Allele GT;
+    Allele Symbolic;
 
     private GenomeLocParser genomeLocParser;
 
@@ -63,6 +64,7 @@ public class GATKVariantContextUtilsUnitTest extends BaseTest {
         ATref = Allele.create("AT",true);
         Anoref = Allele.create("A",false);
         GT = Allele.create("GT",false);
+        Symbolic = Allele.create("<Symbolic>", false);
         genomeLocParser = new GenomeLocParser(new CachingIndexedFastaSequenceFile(new File(hg18Reference)));
     }
 
@@ -1607,5 +1609,25 @@ public class GATKVariantContextUtilsUnitTest extends BaseTest {
         BaseUtils.fillWithRandomBases(bases, 1, bases.length);
         return bases;
     }
+
+    @Test
+    public void testCreateAlleleMapping(){
+        final List<Allele> alleles = Arrays.asList(Aref,Symbolic,T);
+        final VariantContext vc = new VariantContextBuilder().chr("chr1").alleles(alleles).make();
+        Map<Allele, Allele> map = GATKVariantContextUtils.createAlleleMapping(ATref, vc, alleles);
+
+        final List<Allele> expectedAlleles = Arrays.asList(Allele.create("<Symbolic>", false), Allele.create("TT", false));
+        for ( int i = 0; i < vc.getAlternateAlleles().size(); i++ ){
+            Assert.assertEquals(map.get(vc.getAlternateAlleles().get(i)), expectedAlleles.get(i));
+        }
+    }
+
+    @Test(expectedExceptions = IllegalStateException.class)
+    public void testCreateAlleleMappingException(){
+        final List<Allele> alleles = Arrays.asList(Aref, Symbolic, T);
+        final VariantContext vc = new VariantContextBuilder().chr("chr1").alleles(alleles).make();
+        // Throws an exception if the ref allele length <= ref allele length to extend
+        Map<Allele, Allele> map = GATKVariantContextUtils.createAlleleMapping(Aref, vc, alleles);
+    }
 }