From d86ab2becb976b2d5722812fdb4d0a64947a6146 Mon Sep 17 00:00:00 2001 From: depristo Date: Wed, 17 Nov 2010 16:08:16 +0000 Subject: [PATCH] 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 --- .../variantcontext/VariantContextUtils.java | 27 ++++++++------ .../variantcontext/VariantJEXLContext.java | 37 +++++++------------ .../filters/VariantFiltrationWalker.java | 4 +- .../varianteval/VariantEvalWalker.java | 18 ++++++++- .../walkers/variantutils/SelectVariants.java | 23 +----------- .../VariantJEXLContextUnitTest.java | 2 +- 6 files changed, 49 insertions(+), 62 deletions(-) diff --git a/java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantContextUtils.java b/java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantContextUtils.java index 230d1b892..ff3a51c92 100755 --- a/java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantContextUtils.java +++ b/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) { - return match(genomeLocParser,vc,Arrays.asList(exp)).get(exp); + public static boolean match(VariantContext vc, JexlVCMatchExp 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 match(GenomeLocParser genomeLocParser,VariantContext vc, Collection exps) { - return new JEXLMap(genomeLocParser,exps,vc); + public static Map match(VariantContext vc, Collection exps) { + 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) { - return match(genomeLocParser,vc,g,Arrays.asList(exp)).get(exp); + public static boolean match(VariantContext vc, Genotype g, JexlVCMatchExp 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 match(GenomeLocParser genomeLocParser,VariantContext vc, Genotype g, Collection exps) { - return new JEXLMap(genomeLocParser,exps,vc,g); - + public static Map match(VariantContext vc, Genotype g, Collection exps) { + 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); } diff --git a/java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContext.java b/java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContext.java index f36cf3bf6..fce94b758 100644 --- a/java/src/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContext.java +++ b/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) { - this.genomeLocParser = genomeLocParser; + public VariantJEXLContext(VariantContext vc) { 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 { - 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 { private Map jexl; - public JEXLMap(GenomeLocParser genomeLocParser,Collection jexlCollection, VariantContext vc, Genotype g) { - this.genomeLocParser = genomeLocParser; + public JEXLMap(Collection jexlCollection, VariantContext vc, Genotype g) { this.vc = vc; this.g = g; initialize(jexlCollection); } - public JEXLMap(GenomeLocParser genomeLocParser,Collection jexlCollection, VariantContext vc) { - this(genomeLocParser,jexlCollection, vc, null); + public JEXLMap(Collection jexlCollection, VariantContext vc) { + this(jexlCollection, vc, null); } - public JEXLMap(GenomeLocParser genomeLocParser,Collection jexlCollection, Genotype g) { - this(genomeLocParser,jexlCollection, null, g); + public JEXLMap(Collection jexlCollection, Genotype g) { + this(jexlCollection, null, g); } private void initialize(Collection jexlCollection) { @@ -164,14 +154,17 @@ class JEXLMap implements Map { 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 infoMap = new HashMap(); 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("POS", String.valueOf(VariantContextUtils.getLocation(genomeLocParser,vc).getStart())); + infoMap.put("CHROM", vc.getChr()); + 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 { // addAttributesToMap(infoMap, g.getAttributes(), prefix); // infoMap.put(prefix + "GT", g.getGenotypeString()); // } - } - // add specific genotype if one is provided - if ( g != null ) { + // add specific genotype if one is provided 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 { infoMap.put(VCFConstants.GENOTYPE_QUALITY_KEY, new Double(g.getPhredScaledQual())); for ( Map.Entry 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()); } } diff --git a/java/src/org/broadinstitute/sting/gatk/walkers/filters/VariantFiltrationWalker.java b/java/src/org/broadinstitute/sting/gatk/walkers/filters/VariantFiltrationWalker.java index 3ddb0d2da..e8d6bc76c 100755 --- a/java/src/org/broadinstitute/sting/gatk/walkers/filters/VariantFiltrationWalker.java +++ b/java/src/org/broadinstitute/sting/gatk/walkers/filters/VariantFiltrationWalker.java @@ -188,7 +188,7 @@ public class VariantFiltrationWalker extends RodWalker { Set filters = new LinkedHashSet(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 { 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); } diff --git a/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java b/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java index bc9a4d4b0..a813192df 100755 --- a/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java +++ b/java/src/org/broadinstitute/sting/gatk/walkers/varianteval/VariantEvalWalker.java @@ -620,6 +620,7 @@ public class VariantEvalWalker extends RodWalker implements Tr } } + private static Set seenJEXLExceptions = new HashSet(); private boolean applyVCtoEvaluation(VariantContext vc, Map vcs, EvaluationContext group) { if ( vc == null ) return true; @@ -643,8 +644,21 @@ public class VariantEvalWalker extends RodWalker implements Tr else if ( group.requiresNovel() && vcKnown ) return false; - if ( group.selectExp != null && ! VariantContextUtils.match(getToolkit().getGenomeLocParser(),vc, group.selectExp) ) - return false; + if ( group.selectExp != null ) { + 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; diff --git a/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java b/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java index 8addc10d8..db8583cfd 100755 --- a/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java +++ b/java/src/org/broadinstitute/sting/gatk/walkers/variantutils/SelectVariants.java @@ -169,27 +169,6 @@ public class SelectVariants extends RodWalker { 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 { 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; } } diff --git a/java/test/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContextUnitTest.java b/java/test/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContextUnitTest.java index ffd31c9cd..6fce044c9 100644 --- a/java/test/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContextUnitTest.java +++ b/java/test/org/broadinstitute/sting/gatk/contexts/variantcontext/VariantJEXLContextUnitTest.java @@ -146,7 +146,7 @@ public class VariantJEXLContextUnitTest extends BaseTest { List 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); }