JEXL expressions now generate exceptions, not warnings. Tools should catch the runtime exception to handle correctly. Removed unncessary complexity from the JEXL contexts

git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@4695 348d0f76-0448-11de-a6fe-93d51630548a

java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantContextUtils.java

@@ -39,6 +39,10 @@ import org.broadinstitute.sting.utils.exceptions.UserException;
 
 public class VariantContextUtils {
     final public static JexlEngine engine = new JexlEngine();
+    static {
+        engine.setSilent(false); // will throw errors now for selects that don't evaluate properly
+        engine.setLenient(false);
+    }
 
     /**
      * Create a new VariantContext
@@ -211,7 +215,7 @@ public class VariantContextUtils {
                 Expression exp = engine.createExpression(expStr);
                 exps.add(new JexlVCMatchExp(name, exp));
             } catch (Exception e) {
-		throw new UserException.BadArgumentValue(name, "Invalid expression used (" + expStr + "). Please see the JEXL docs for correct syntax.") ;
+                throw new UserException.BadArgumentValue(name, "Invalid expression used (" + expStr + "). Please see the JEXL docs for correct syntax.") ;
             }
         }
 
@@ -224,8 +228,8 @@ public class VariantContextUtils {
      * @param exp   expression
      * @return true if there is a match
      */
-    public static boolean match(GenomeLocParser genomeLocParser,VariantContext vc, JexlVCMatchExp exp) {
+    public static boolean match(VariantContext vc, JexlVCMatchExp exp) {
-        return match(genomeLocParser,vc,Arrays.asList(exp)).get(exp);
+        return match(vc,Arrays.asList(exp)).get(exp);
     }
 
     /**
@@ -238,8 +242,8 @@ public class VariantContextUtils {
      * @param exps expressions
      * @return true if there is a match
      */
-    public static Map<JexlVCMatchExp, Boolean> match(GenomeLocParser genomeLocParser,VariantContext vc, Collection<JexlVCMatchExp> exps) {
+    public static Map<JexlVCMatchExp, Boolean> match(VariantContext vc, Collection<JexlVCMatchExp> exps) {
-        return new JEXLMap(genomeLocParser,exps,vc);
+        return new JEXLMap(exps,vc);
 
     }
 
@@ -250,8 +254,8 @@ public class VariantContextUtils {
      * @param exp   expression
      * @return true if there is a match
      */
-    public static boolean match(GenomeLocParser genomeLocParser,VariantContext vc, Genotype g, JexlVCMatchExp exp) {
+    public static boolean match(VariantContext vc, Genotype g, JexlVCMatchExp exp) {
-        return match(genomeLocParser,vc,g,Arrays.asList(exp)).get(exp);
+        return match(vc,g,Arrays.asList(exp)).get(exp);
     }
 
     /**
@@ -265,9 +269,8 @@ public class VariantContextUtils {
      * @param exps expressions
      * @return true if there is a match
      */
-    public static Map<JexlVCMatchExp, Boolean> match(GenomeLocParser genomeLocParser,VariantContext vc, Genotype g, Collection<JexlVCMatchExp> exps) {
+    public static Map<JexlVCMatchExp, Boolean> match(VariantContext vc, Genotype g, Collection<JexlVCMatchExp> exps) {
-        return new JEXLMap(genomeLocParser,exps,vc,g);
+        return new JEXLMap(exps,vc,g);
-
     }
 
     public static double computeHardyWeinbergPvalue(VariantContext vc) {
@@ -354,7 +357,7 @@ public class VariantContextUtils {
         for (VariantContext vc : prepaddedVCs) {
             // also a reasonable place to remove filtered calls, if needed
             if ( ! filteredAreUncalled || vc.isNotFiltered() )
-                VCs.add(VariantContext.createVariantContextWithPaddedAlleles(vc,inputRefBase,false));            
+                VCs.add(VariantContext.createVariantContextWithPaddedAlleles(vc,inputRefBase,false));
         }
         if ( VCs.size() == 0 ) // everything is filtered out and we're filteredareUncalled
             return null;
@@ -454,7 +457,7 @@ public class VariantContextUtils {
                         s.add( vc.isFiltered() ? "filterIn" + vc.getSource() : vc.getSource() );
                 setValue = Utils.join("-", s);
             }
-            
+
             if ( setKey != null ) attributes.put(setKey, setValue);
         }
 

java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContext.java

@@ -27,7 +27,6 @@ import org.apache.commons.jexl2.JexlContext;
 import org.apache.commons.jexl2.MapContext;
 import org.broad.tribble.util.variantcontext.Genotype;
 import org.broad.tribble.util.variantcontext.VariantContext;
-import org.broadinstitute.sting.utils.GenomeLocParser;
 import org.broadinstitute.sting.utils.Utils;
 import org.broad.tribble.vcf.VCFConstants;
 import org.broadinstitute.sting.utils.exceptions.UserException;
@@ -50,7 +49,6 @@ import java.util.*;
  */
 
 class VariantJEXLContext implements JexlContext {
-    private GenomeLocParser genomeLocParser;
     // our stored variant context
     private VariantContext vc;
 
@@ -75,16 +73,10 @@ class VariantJEXLContext implements JexlContext {
         x.put("homVarCount",  new AttributeGetter() { public Object get(VariantContext vc) { return vc.getHomVarCount(); }});
     }
 
-    public VariantJEXLContext(GenomeLocParser genomeLocParser,VariantContext vc) {
+    public VariantJEXLContext(VariantContext vc) {
-        this.genomeLocParser = genomeLocParser;
         this.vc = vc;
     }
 
-//    public VariantJEXLContext(VariantContext vc, Genotype g) {
-//        this.vc = vc;
-//        //throw new UnsupportedOperationException("Cannot instantiate VariantJEXLContext");
-//    }
-
     public Object get(String name) {
         Object result = null;
         if ( x.containsKey(name) ) { // dynamic resolution of name -> value via map
@@ -122,7 +114,6 @@ class VariantJEXLContext implements JexlContext {
  */
 
 class JEXLMap implements Map<VariantContextUtils.JexlVCMatchExp, Boolean> {
-    private final GenomeLocParser genomeLocParser;
     // our variant context and/or Genotype
     private final VariantContext vc;
     private final Genotype g;
@@ -134,19 +125,18 @@ class JEXLMap implements Map<VariantContextUtils.JexlVCMatchExp, Boolean> {
     private Map<VariantContextUtils.JexlVCMatchExp,Boolean> jexl;
 
 
-    public JEXLMap(GenomeLocParser genomeLocParser,Collection<VariantContextUtils.JexlVCMatchExp> jexlCollection, VariantContext vc, Genotype g) {
+    public JEXLMap(Collection<VariantContextUtils.JexlVCMatchExp> jexlCollection, VariantContext vc, Genotype g) {
-        this.genomeLocParser = genomeLocParser;
         this.vc = vc;
         this.g = g;
         initialize(jexlCollection);
     }
 
-    public JEXLMap(GenomeLocParser genomeLocParser,Collection<VariantContextUtils.JexlVCMatchExp> jexlCollection, VariantContext vc) {
+    public JEXLMap(Collection<VariantContextUtils.JexlVCMatchExp> jexlCollection, VariantContext vc) {
-        this(genomeLocParser,jexlCollection, vc, null);
+        this(jexlCollection, vc, null);
     }
 
-    public JEXLMap(GenomeLocParser genomeLocParser,Collection<VariantContextUtils.JexlVCMatchExp> jexlCollection, Genotype g) {
+    public JEXLMap(Collection<VariantContextUtils.JexlVCMatchExp> jexlCollection, Genotype g) {
-        this(genomeLocParser,jexlCollection, null, g);
+        this(jexlCollection, null, g);
     }
 
     private void initialize(Collection<VariantContextUtils.JexlVCMatchExp> jexlCollection) {
@@ -164,14 +154,17 @@ class JEXLMap implements Map<VariantContextUtils.JexlVCMatchExp, Boolean> {
     private void createContext() {
         if ( g == null ) {
             // todo -- remove dependancy on g to the entire system
-            jContext = new VariantJEXLContext(genomeLocParser,vc);
+            jContext = new VariantJEXLContext(vc);
         } else {
+            //
+            // this whole branch is here just to support G jexl operations
+            //
             Map<String, Object> infoMap = new HashMap<String, Object>();
 
             if ( vc != null ) {
                 // create a mapping of what we know about the variant context, its Chromosome, positions, etc.
-                infoMap.put("CHROM", VariantContextUtils.getLocation(genomeLocParser,vc).getContig());
+                infoMap.put("CHROM", vc.getChr());
-                infoMap.put("POS", String.valueOf(VariantContextUtils.getLocation(genomeLocParser,vc).getStart()));
+                infoMap.put("POS", vc.getStart());
                 infoMap.put("TYPE", vc.getType().toString());
                 infoMap.put("QUAL", String.valueOf(vc.getPhredScaledQual()));
 
@@ -195,10 +188,8 @@ class JEXLMap implements Map<VariantContextUtils.JexlVCMatchExp, Boolean> {
                 //        addAttributesToMap(infoMap, g.getAttributes(), prefix);
                 //        infoMap.put(prefix + "GT", g.getGenotypeString());
                 //    }
-            }
 
-            // add specific genotype if one is provided
+                // add specific genotype if one is provided
-            if ( g != null ) {
                 infoMap.put(VCFConstants.GENOTYPE_KEY, g.getGenotypeString());
                 infoMap.put("isHomRef", g.isHomRef() ? "1" : "0");
                 infoMap.put("isHet", g.isHet() ? "1" : "0");
@@ -206,7 +197,7 @@ class JEXLMap implements Map<VariantContextUtils.JexlVCMatchExp, Boolean> {
                 infoMap.put(VCFConstants.GENOTYPE_QUALITY_KEY, new Double(g.getPhredScaledQual()));
                 for ( Map.Entry<String, Object> e : g.getAttributes().entrySet() ) {
                     if ( e.getValue() != null && !e.getValue().equals(VCFConstants.MISSING_VALUE_v4) )
-                    infoMap.put(e.getKey(), e.getValue());
+                        infoMap.put(e.getKey(), e.getValue());
                 }
             }
 

java/src/org/broadinstitute/sting/gatk/walkers/filters/VariantFiltrationWalker.java

@@ -188,7 +188,7 @@ public class VariantFiltrationWalker extends RodWalker<Integer, Integer> {
                     Set<String> filters = new LinkedHashSet<String>(g.getFilters());
 
                     for ( VariantContextUtils.JexlVCMatchExp exp : genotypeFilterExps ) {
-                        if ( VariantContextUtils.match(getToolkit().getGenomeLocParser(),vc, g, exp) )
+                        if ( VariantContextUtils.match(vc, g, exp) )
                             filters.add(exp.name);
                     }
                     genotypes.put(genotype.getKey(), new Genotype(genotype.getKey(), g.getAlleles(), g.getNegLog10PError(), filters, g.getAttributes(), g.genotypesArePhased()));
@@ -211,7 +211,7 @@ public class VariantFiltrationWalker extends RodWalker<Integer, Integer> {
             filters.add(CLUSTERED_SNP_FILTER_NAME);
 
         for ( VariantContextUtils.JexlVCMatchExp exp : filterExps ) {
-            if ( VariantContextUtils.match(getToolkit().getGenomeLocParser(),vc, exp) )
+            if ( VariantContextUtils.match(vc, exp) )
                 filters.add(exp.name);
         }
 

java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java

@@ -620,6 +620,7 @@ public class VariantEvalWalker extends RodWalker<Integer, Integer> implements Tr
         }
     }
 
+    private static Set<String> seenJEXLExceptions = new HashSet<String>();
     private boolean applyVCtoEvaluation(VariantContext vc, Map<String, VariantContext> vcs, EvaluationContext group) {
         if ( vc == null )
             return true;
@@ -643,8 +644,21 @@ public class VariantEvalWalker extends RodWalker<Integer, Integer> implements Tr
         else if ( group.requiresNovel() && vcKnown )
             return false;
 
-        if ( group.selectExp != null && ! VariantContextUtils.match(getToolkit().getGenomeLocParser(),vc, group.selectExp) )
+        if ( group.selectExp != null ) {
-            return false;
+            try {
+                if ( ! VariantContextUtils.match(vc, group.selectExp) )
+                    return false;
+            } catch ( RuntimeException e ) {
+                if ( ! seenJEXLExceptions.contains(group.selectExp.name) ) {
+                    seenJEXLExceptions.add(group.selectExp.name);
+                    logger.warn("JEXL evaluation error for SELECT " + group.selectExp.name + ": " + e.getMessage() +
+                            "; this may be an error or may simply result from some variants not having INFO fields keys " +
+                            "referenced in the JEXL expressions.  Variants generating exceptions will *NOT* be matched " +
+                            "by the expression.  Occurred with variant " + vc);
+                }
+                return false;
+            }
+        }
 
         // nothing invalidated our membership in this set
         return true;

java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java

@@ -169,27 +169,6 @@ public class SelectVariants extends RodWalker<Integer, Integer> {
         jexls = VariantContextUtils.initializeMatchExps(selectNames, SELECT_EXPRESSIONS);
     }
 
-//    This is commented out rather than just deleted in case I want to enable JEXL matching *before* sample subsetting.
-//    /**
-//     * If JEXL expressions are supplied, include only records that satisfy the expression
-//     *
-//     * @param  tracker   the ROD tracker
-//     * @param  ref       reference information
-//     * @param  context   alignment info
-//     * @return true if no JEXL expressions are supplied or if a record satisfies all JEXL criteria, false if otherwise
-//     */
-//    public boolean filter(RefMetaDataTracker tracker, ReferenceContext ref, AlignmentContext context) {
-//        VariantContext vc = tracker.getVariantContext(ref, "variant", null, context.getLocation(), true);
-//
-//        for ( VariantContextUtils.JexlVCMatchExp jexl : jexls ) {
-//            if ( !VariantContextUtils.match(vc, jexl) ) {
-//                return false;
-//            }
-//        }
-//
-//        return true;
-//    }
-
     /**
      * Subset VC record if necessary and emit the modified record (provided it satisfies criteria for printing)
      *
@@ -214,7 +193,7 @@ public class SelectVariants extends RodWalker<Integer, Integer> {
             if ( (sub.isPolymorphic() || !EXCLUDE_NON_VARIANTS) && (!sub.isFiltered() || !EXCLUDE_FILTERED) ) {
                     //System.out.printf("%s%n",sub.toString());
                 for ( VariantContextUtils.JexlVCMatchExp jexl : jexls ) {
-                    if ( !VariantContextUtils.match(getToolkit().getGenomeLocParser(),sub, jexl) ) {
+                    if ( !VariantContextUtils.match(sub, jexl) ) {
                         return 0;
                     }
                 }

java/test/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContextUnitTest.java

@@ -146,7 +146,7 @@ public class VariantJEXLContextUnitTest extends BaseTest {
         List<Allele> alleles = Arrays.asList(Aref, T);
 
         VariantContext vc = new VariantContext("test", snpLoc.getContig(), snpLoc.getStart(), snpLoc.getStop(), alleles);
-        return new JEXLMap(genomeLocParser,Arrays.asList(exp),vc);
+        return new JEXLMap(Arrays.asList(exp),vc);
     }