zzh
/
gatk-3.8
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improved detection of missing argument values 

In particular, it was possible to specify arguments for Files or Compound types without values
 Added a special "none" value for annotations, since a bare "-A" is no longer allowed
 Delivers PT 71792842 and 59360374
This commit is contained in:
Phillip Dexheimer 2014-07-07 22:48:53 -04:00
parent 03e7ee6e9c
commit 593663d9b6
11 changed files with 93 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/genotyper/UnifiedGenotyper.java
View File

@ -212,7 +212,7 @@ public class UnifiedGenotyper extends LocusWalker<List<VariantCallContext>, Unif
* If specified, all available annotations in the group will be applied. See the VariantAnnotator -list argument to view available groups.
* Keep in mind that RODRequiringAnnotations are not intended to be used as a group, because they require specific ROD inputs.
*/
@Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls", required=false)
@Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls. The single value 'none' removes the default group", required=false)
protected String[] annotationClassesToUse = { "Standard" };
// the calculation arguments

4
protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCaller.java
View File

@ -320,7 +320,7 @@ public class HaplotypeCaller extends ActiveRegionWalker<List<VariantContext>, In
* Which annotations to add to the output VCF file. See the VariantAnnotator -list argument to view available annotations.
*/
@Advanced
@Argument(fullName="annotation", shortName="A", doc="One or more specific annotations to apply to variant calls", required=false)
@Argument(fullName="annotation", shortName="A", doc="One or more specific annotations to apply to variant calls. The single value 'none' removes the default annotations", required=false)
protected List<String> annotationsToUse = new ArrayList<>(Arrays.asList(new String[]{"ClippingRankSumTest", "DepthPerSampleHC"}));
/**
@ -334,7 +334,7 @@ public class HaplotypeCaller extends ActiveRegionWalker<List<VariantContext>, In
/**
* Which groups of annotations to add to the output VCF file. See the VariantAnnotator -list argument to view available groups.
*/
@Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls", required=false)
@Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls. The single value 'none' removes the default group", required=false)
protected String[] annotationClassesToUse = { "Standard" };
@ArgumentCollection

2
protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/variantutils/GenotypeGVCFs.java
View File

@ -133,7 +133,7 @@ public class GenotypeGVCFs extends RodWalker<VariantContext, VariantContextWrite
* Which annotations to recompute for the combined output VCF file.
*/
@Advanced
@Argument(fullName="annotation", shortName="A", doc="One or more specific annotations to recompute", required=false)
@Argument(fullName="annotation", shortName="A", doc="One or more specific annotations to recompute. The single value 'none' removes the default annotations", required=false)
protected List<String> annotationsToUse = new ArrayList<>(Arrays.asList(new String[]{"InbreedingCoeff", "FisherStrand", "QualByDepth", "ChromosomeCounts", "GenotypeSummaries"}));
/**

2
protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCallerParallelIntegrationTest.java
View File

@ -71,7 +71,7 @@ public class HaplotypeCallerParallelIntegrationTest extends WalkerTest {
WalkerTestSpec spec = new WalkerTestSpec(
"-T HaplotypeCaller --pcr_indel_model NONE -R " + b37KGReference + " --no_cmdline_in_header -I "
+ privateTestDir + "PCRFree.2x250.Illumina.20_10_11.bam -o %s " +
" -L 20:10,000,000-10,100,000 -G none -A -contamination 0.0 -nct " + nct, 1,
" -L 20:10,000,000-10,100,000 -G none -A none -contamination 0.0 -nct " + nct, 1,
Arrays.asList(md5));
executeTest("HC test parallel HC with NCT with nct " + nct, spec);
}

2
protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/utils/nanoScheduler/NanoSchedulerIntegrationTest.java
View File

@ -78,7 +78,7 @@ public class NanoSchedulerIntegrationTest extends WalkerTest {
WalkerTestSpec spec = new WalkerTestSpec(
buildCommandLine(
"-T UnifiedGenotyper -R " + b37KGReference,
"--no_cmdline_in_header -G",
"--no_cmdline_in_header -G none",
//"--dbsnp " + b37dbSNP132,
"-I " + privateTestDir + "NA12878.HiSeq.b37.chr20.10_11mb.bam",
"-L 20:10,000,000-10,100,000",

41
public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/tools/walkers/annotator/interfaces/AnnotationInterfaceManager.java
View File

@ -32,6 +32,9 @@ import org.broadinstitute.gatk.utils.exceptions.UserException;
import java.util.*;
public class AnnotationInterfaceManager {
private static final String NULL_ANNOTATION_NAME = "none";
private static final String NULL_ANNOTATION_GROUP_NAME = "none";
private static PluginManager<InfoFieldAnnotation> infoFieldAnnotationPluginManager = new PluginManager<InfoFieldAnnotation>(InfoFieldAnnotation.class);
private static PluginManager<GenotypeAnnotation> genotypeAnnotationPluginManager = new PluginManager<GenotypeAnnotation>(GenotypeAnnotation.class);
private static PluginManager<AnnotationType> annotationTypePluginManager = new PluginManager<AnnotationType>(AnnotationType.class);
@ -53,7 +56,7 @@ public class AnnotationInterfaceManager {
for ( Class c : annotationTypePluginManager.getInterfaces() )
classMap.put(c.getSimpleName(), c);
if ( annotationGroupsToUse.size() != 1 || !"none".equals(annotationGroupsToUse.get(0)) ) {
if ( annotationGroupsToUse.size() != 1 || !NULL_ANNOTATION_GROUP_NAME.equals(annotationGroupsToUse.get(0)) ) {
for ( String group : annotationGroupsToUse ) {
Class interfaceClass = classMap.get(group);
if ( interfaceClass == null )
@ -64,15 +67,17 @@ public class AnnotationInterfaceManager {
}
// validate the specific classes provided
for ( String annotation : annotationsToUse ) {
Class annotationClass = classMap.get(annotation);
if ( annotationClass == null )
annotationClass = classMap.get(annotation + "Annotation");
if ( annotationClass == null ) {
if (DeprecatedToolChecks.isDeprecatedAnnotation(annotation) ) {
throw new UserException.DeprecatedAnnotation(annotation, DeprecatedToolChecks.getAnnotationDeprecationInfo(annotation));
} else {
throw new UserException.BadArgumentValue("annotation", "Annotation " + annotation + " was not found; please check that you have specified the annotation name correctly");
if ( annotationsToUse.size() != 1 || !NULL_ANNOTATION_NAME.equals(annotationsToUse.get(0)) ) {
for (String annotation : annotationsToUse) {
Class annotationClass = classMap.get(annotation);
if (annotationClass == null)
annotationClass = classMap.get(annotation + "Annotation");
if (annotationClass == null) {
if (DeprecatedToolChecks.isDeprecatedAnnotation(annotation)) {
throw new UserException.DeprecatedAnnotation(annotation, DeprecatedToolChecks.getAnnotationDeprecationInfo(annotation));
} else {
throw new UserException.BadArgumentValue("annotation", "Annotation " + annotation + " was not found; please check that you have specified the annotation name correctly");
}
}
}
}
@ -105,7 +110,7 @@ public class AnnotationInterfaceManager {
}
});
if ( annotationGroupsToUse.size() != 1 || !"none".equals(annotationGroupsToUse.get(0)) ) {
if ( annotationGroupsToUse.size() != 1 || !NULL_ANNOTATION_GROUP_NAME.equals(annotationGroupsToUse.get(0)) ) {
for ( String group : annotationGroupsToUse ) {
Class interfaceClass = classMap.get(group);
if ( interfaceClass == null )
@ -116,12 +121,14 @@ public class AnnotationInterfaceManager {
}
// get the specific classes provided
for ( String annotation : annotationsToUse ) {
Class annotationClass = classMap.get(annotation);
if ( annotationClass == null )
annotationClass = classMap.get(annotation + "Annotation");
if ( annotationClass != null )
classes.add(annotationClass);
if ( annotationsToUse.size() != 1 || !NULL_ANNOTATION_NAME.equals(annotationsToUse.get(0)) ) {
for (String annotation : annotationsToUse) {
Class annotationClass = classMap.get(annotation);
if (annotationClass == null)
annotationClass = classMap.get(annotation + "Annotation");
if (annotationClass != null)
classes.add(annotationClass);
}
}
// note that technically an annotation can work on both the INFO and FORMAT fields

5
public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentDefinitions.java
View File

@ -174,8 +174,9 @@ public class ArgumentDefinitions implements Iterable<ArgumentDefinition> {
static DefinitionMatcher VerifiableDefinitionMatcher = new DefinitionMatcher() {
public boolean matches( ArgumentDefinition definition, Object key ) {
// We can perform some sort of validation for anything that isn't a flag.
return !definition.isFlag;
// We can perform some sort of validation for anything that isn't a flag or enum.
// Because enums can have a default value, it might be valid to specify an enum argument with no value
return !definition.isFlag && !definition.argumentType.isEnum();
}
};
}

12
public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentMatch.java
View File

@ -187,7 +187,7 @@ public class ArgumentMatch implements Iterable<ArgumentMatch> {
* @return True if there's another token waiting in the wings. False otherwise.
*/
public boolean hasNext() {
return nextToken != null;
return nextSite != null;
}
/**
@ -195,7 +195,7 @@ public class ArgumentMatch implements Iterable<ArgumentMatch> {
* @return The next ArgumentMatch in the series. Should never be null.
*/
public ArgumentMatch next() {
if( nextSite == null || nextToken == null )
if( nextSite == null )
throw new IllegalStateException( "No more ArgumentMatches are available" );
ArgumentMatch match = new ArgumentMatch( label, definition, nextSite, nextToken, tags );
@ -221,10 +221,8 @@ public class ArgumentMatch implements Iterable<ArgumentMatch> {
nextSite = siteIterator.next();
if( sites.get(nextSite) != null ) {
tokenIterator = sites.get(nextSite).iterator();
if( tokenIterator.hasNext() ) {
nextToken = tokenIterator.next();
break;
}
nextToken = tokenIterator.hasNext() ? tokenIterator.next() : null;
break;
}
}
}
@ -280,6 +278,8 @@ public class ArgumentMatch implements Iterable<ArgumentMatch> {
for ( final List<ArgumentMatchValue> siteValue : sites.values() ) {
if ( siteValue != null )
values.addAll(siteValue);
else
values.add(null);
}
return values;
}

4
public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentTypeDescriptor.java
View File

@ -736,8 +736,10 @@ class SimpleArgumentTypeDescriptor extends ArgumentTypeDescriptor {
} else if (type.equals(File.class)) {
result = value == null ? null : value.asFile();
} else {
if (value == null)
throw new MissingArgumentValueException(createDefaultArgumentDefinition(source));
Constructor ctor = type.getConstructor(String.class);
result = ctor.newInstance(value == null ? null : value.asString());
result = ctor.newInstance(value.asString());
}
} catch (UserException e) {
throw e;

2
public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ParsingEngine.java
View File

@ -317,7 +317,7 @@ public class ParsingEngine {
// when the argument name is specified and the argument is not a flag type.
for(ArgumentMatch verifiableMatch: verifiableMatches) {
ArgumentSource argumentSource = argumentSourcesByDefinition.get(verifiableArgument);
if(verifiableMatch.values().size() == 0 && !verifiableArgument.isFlag && argumentSource.createsTypeDefault())
if(verifiableMatch.values().size() == 0 && !verifiableArgument.isFlag && !argumentSource.createsTypeDefault())
invalidValues.add(new Pair<ArgumentDefinition,String>(verifiableArgument,null));
}

51
public/gatk-tools-public/src/test/java/org/broadinstitute/gatk/utils/commandline/ParsingEngineUnitTest.java
View File

@ -40,6 +40,8 @@ import java.io.File;
import java.io.IOException;
import java.util.List;
import java.util.EnumSet;
import java.util.Set;
/**
* Test suite for the parsing engine.
*/
@ -135,7 +137,7 @@ public class ParsingEngineUnitTest extends BaseTest {
Assert.assertEquals(argProvider.foo, 5, "Argument is not correctly initialized");
}
@Test(expectedExceptions=MissingArgumentValueException.class)
@Test(expectedExceptions=InvalidArgumentValueException.class)
public void primitiveArgumentNoValueTest() {
final String[] commandLine = new String[] {"--foo"};
@ -948,6 +950,7 @@ public class ParsingEngineUnitTest extends BaseTest {
}
}
@SuppressWarnings("unused")
private class NumericRangeArgProvider {
@Argument(fullName = "intWithHardMinAndMax", minValue = 5, maxValue = 10)
public int intWithHardMinAndMax;
@ -1089,4 +1092,50 @@ public class ParsingEngineUnitTest extends BaseTest {
NumericRangeArgProvider argProvider = new NumericRangeArgProvider();
parsingEngine.loadArgumentsIntoObject(argProvider);
}
@SuppressWarnings("unused")
private class VariedTypeArgProvider {
@Argument(fullName = "intVal", required=false)
private int anInt;
@Argument(fullName = "stringVal", required=false)
private String aString;
@Argument(fullName = "enumVal", required=false)
private TestEnum anEnum;
@Argument(fullName = "fileVal", required=false)
private File aFile;
@Argument(fullName = "stringSet", required=false)
private Set<String> someStrings;
@Argument(fullName = "intervalVal", required=false)
private IntervalBinding<Feature> anInterval;
}
@DataProvider(name = "MissingArgumentValueDataProvider")
public Object[][] missingArgumentDataProvider() {
return new Object[][]{
{ new String[]{"--intVal"} },
{ new String[]{"--stringVal"} },
{ new String[]{"--enumVal"} },
{ new String[]{"--fileVal"} },
{ new String[]{"--stringSet"} },
{ new String[]{"--stringSet", "aha", "--stringSet"} },
{ new String[]{"--intervalVal"} }
};
}
@Test(dataProvider = "MissingArgumentValueDataProvider",
expectedExceptions = {InvalidArgumentValueException.class, MissingArgumentValueException.class})
public void testMissingArguments( final String[] commandLine ) {
parsingEngine.addArgumentSource( VariedTypeArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.validate();
VariedTypeArgProvider argProvider = new VariedTypeArgProvider();
parsingEngine.loadArgumentsIntoObject(argProvider);
}
}