From 5d6a6420a9e6b76d6125398e93781d93301e819a Mon Sep 17 00:00:00 2001 From: hanna Date: Mon, 23 Aug 2010 23:39:13 +0000 Subject: [PATCH] New behavior for filling it output streams: if required==true for a field and the field is an output stream, we'll automatically create it and point it to stdout. Otherwise, we'll leave it empty. I think about it like this: marking a field 'required' indicates to the GATK that the walker author requires a value for this field, and if the GATK can provide one without end user intervention, it will. Maybe this is hackish. We'll try it and see. git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@4091 348d0f76-0448-11de-a6fe-93d51630548a --- .../sting/commandline/Argument.java | 7 +- .../sting/commandline/ArgumentDefinition.java | 108 +++++------------- .../sting/commandline/ArgumentSource.java | 9 ++ .../commandline/ArgumentTypeDescriptor.java | 18 +-- .../sting/commandline/CommandLineUtils.java | 17 ++- .../sting/commandline/Input.java | 7 +- .../sting/commandline/Output.java | 9 +- .../sting/commandline/ParsingEngine.java | 2 +- .../SAMFileWriterArgumentTypeDescriptor.java | 9 +- .../walkers/genotyper/UnifiedGenotyper.java | 20 +--- 10 files changed, 89 insertions(+), 117 deletions(-) diff --git a/java/src/org/broadinstitute/sting/commandline/Argument.java b/java/src/org/broadinstitute/sting/commandline/Argument.java index 557fbe34e..b2ee9d1fc 100755 --- a/java/src/org/broadinstitute/sting/commandline/Argument.java +++ b/java/src/org/broadinstitute/sting/commandline/Argument.java @@ -45,7 +45,7 @@ import java.lang.annotation.Target; @Documented @Inherited @Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.FIELD}) +@Target(ElementType.FIELD) public @interface Argument { /** * The full name of the command-line argument. Full names should be @@ -70,8 +70,9 @@ public @interface Argument { String doc(); /** - * Is this command-line argument required. The application should exit - * printing help if this command-line argument is not specified. + * Is this argument required. If true, the command-line argument system will + * make a best guess for populating this argument based on the type descriptor, + * and will fail if the type can't be populated. * @return True if the argument is required. False otherwise. */ boolean required() default true; diff --git a/java/src/org/broadinstitute/sting/commandline/ArgumentDefinition.java b/java/src/org/broadinstitute/sting/commandline/ArgumentDefinition.java index 5a48e3c97..1139c1f55 100644 --- a/java/src/org/broadinstitute/sting/commandline/ArgumentDefinition.java +++ b/java/src/org/broadinstitute/sting/commandline/ArgumentDefinition.java @@ -61,7 +61,13 @@ public class ArgumentDefinition { public final String doc; /** - * Is this argument required? + * Must this argument be specified on the command-line? Note that there's a + * critical difference between the meaning of a required argument from the + * perspective of the argument source and the perspective of the argument + * definition: the argument source's required field indicates that the field + * should somehow be populated by the GATK (and fail if there's an error). + * The ArgumentDefinition required element means that the required element + * must be specified on the command-line. */ public final boolean required; @@ -137,8 +143,8 @@ public class ArgumentDefinition { this.required = required; this.isFlag = isFlag; this.isMultiValued = isMultiValued; - this.componentType = componentType; this.isHidden = isHidden; + this.componentType = componentType; this.exclusiveOf = exclusiveOf; this.validation = validation; this.validOptions = validOptions; @@ -157,17 +163,22 @@ public class ArgumentDefinition { * @param validOptions is there a particular list of options that's valid for this argument definition? List them if so, otherwise set this to null. */ public ArgumentDefinition( Annotation annotation, + ArgumentIOType ioType, Class argumentType, String defaultFullName, String defaultShortName, + String doc, + boolean isRequired, boolean isFlag, boolean isMultiValued, boolean isHidden, Class componentType, + String exclusiveOf, + String validation, List validOptions) { - String fullName = (String)getValue(annotation, "fullName"); - String shortName = (String)getValue(annotation, "shortName"); + String fullName = (String)CommandLineUtils.getValue(annotation, "fullName"); + String shortName = (String)CommandLineUtils.getValue(annotation, "shortName"); boolean isFullNameProvided = fullName.trim().length() > 0; boolean isShortNameProvided = shortName.trim().length() > 0; @@ -182,55 +193,21 @@ public class ArgumentDefinition { else shortName = null; - this.ioType = ArgumentIOType.getIOType(annotation); + this.ioType = ioType; this.argumentType = argumentType; this.fullName = fullName; this.shortName = shortName; - this.doc = getDoc(annotation); - this.required = isRequired(annotation, isFlag); + this.doc = doc; + this.required = isRequired; this.isFlag = isFlag; this.isMultiValued = isMultiValued; this.isHidden = isHidden; this.componentType = componentType; - this.exclusiveOf = getExclusiveOf(annotation); - this.validation = getValidationRegex(annotation); + this.exclusiveOf = exclusiveOf; + this.validation = validation; this.validOptions = validOptions; } - /** - * Creates a new argument definition. - * @param annotation The annotation on the field. - * @param argumentType The class of the field. - * @param fieldName Default full name for this argument definition. - * @param isFlag Whether or not this argument should be treated as a flag. - * @param isMultiValued Whether or not this argument supports multiple values. - * @param componentType For multivalued arguments the type of the components. - * @param isHidden Whether or not this argument should be hidden from the command-line argument system. - * @param validOptions is there a particular list of options that's valid for this argument definition? List them if so, otherwise set this to null. - */ - public ArgumentDefinition( Annotation annotation, - Class argumentType, - String fieldName, - boolean isFlag, - boolean isMultiValued, - boolean isHidden, - Class componentType, - List validOptions) { - this.ioType = ArgumentIOType.getIOType(annotation); - this.argumentType = argumentType; - this.fullName = getFullName(annotation, fieldName); - this.shortName = getShortName(annotation); - this.doc = getDoc(annotation); - this.required = isRequired(annotation, isFlag); - this.isFlag = isFlag; - this.isMultiValued = isMultiValued; - this.isHidden = isHidden; - this.componentType = componentType; - this.exclusiveOf = getExclusiveOf(annotation); - this.validation = getValidationRegex(annotation); - this.validOptions = validOptions; - } - @Override public int hashCode() { int hashCode = fullName.hashCode(); @@ -250,20 +227,6 @@ public class ArgumentDefinition { Utils.equals(shortName,other.shortName); } - /** - * A hack to get around the fact that Java doesn't like inheritance in Annotations. - * @param annotation to run the method on - * @param method the method to invoke - * @return the return value of the method - */ - private static Object getValue(Annotation annotation, String method) { - try { - return annotation.getClass().getMethod(method).invoke(annotation); - } catch (Exception e) { - throw new StingException("Unable to access method " + method + " on annotation " + annotation.getClass(), e); - } - } - /** * Retrieves the full name of the argument, specifiable with the '--' prefix. The full name can be * either specified explicitly with the fullName annotation parameter or implied by the field name. @@ -271,8 +234,8 @@ public class ArgumentDefinition { * @param fieldName Original field name. * @return full name of the argument. Never null. */ - private static String getFullName( Annotation annotation, String fieldName ) { - String fullName = (String)getValue(annotation, "fullName"); + public static String getFullName( Annotation annotation, String fieldName ) { + String fullName = (String)CommandLineUtils.getValue(annotation, "fullName"); return fullName.trim().length() > 0 ? fullName.trim() : fieldName.toLowerCase(); } @@ -282,8 +245,8 @@ public class ArgumentDefinition { * @param annotation Original field annotation. * @return short name of the argument. Null if no short name exists. */ - private static String getShortName( Annotation annotation ) { - String shortName = (String)getValue(annotation, "shortName"); + public static String getShortName( Annotation annotation ) { + String shortName = (String)CommandLineUtils.getValue(annotation, "shortName"); return shortName.trim().length() > 0 ? shortName.trim() : null; } @@ -292,19 +255,8 @@ public class ArgumentDefinition { * @param annotation Original field annotation. * @return Documentation for this argument. */ - private static String getDoc( Annotation annotation ) { - return (String)getValue(annotation, "doc"); - } - - /** - * Returns whether this field is required. Note that flag fields are always forced to 'not required'. - * @param annotation Original field annotation. - * @param isFlag True if the field is a flag. - * @return True if the field is mandatory and not a boolean flag. False otherwise. - */ - private static boolean isRequired( Annotation annotation, boolean isFlag ) { - boolean required = (Boolean)getValue(annotation, "required"); - return required && !isFlag; + public static String getDoc( Annotation annotation ) { + return (String)CommandLineUtils.getValue(annotation, "doc"); } /** @@ -312,8 +264,8 @@ public class ArgumentDefinition { * @param annotation Original field annotation. * @return A comma-separated list of exclusive arguments, or null if none are present. */ - private static String getExclusiveOf( Annotation annotation ) { - String exclusiveOf = (String)getValue(annotation, "exclusiveOf"); + public static String getExclusiveOf( Annotation annotation ) { + String exclusiveOf = (String)CommandLineUtils.getValue(annotation, "exclusiveOf"); return exclusiveOf.trim().length() > 0 ? exclusiveOf.trim() : null; } @@ -322,8 +274,8 @@ public class ArgumentDefinition { * @param annotation Original field annotation. * @return a JVM regex-compatible regular expression, or null to permit any possible value. */ - private static String getValidationRegex( Annotation annotation ) { - String validation = (String)getValue(annotation, "validation"); + public static String getValidationRegex( Annotation annotation ) { + String validation = (String)CommandLineUtils.getValue(annotation, "validation"); return validation.trim().length() > 0 ? validation.trim() : null; } } diff --git a/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java b/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java index c26a349fe..9bf41016b 100644 --- a/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java +++ b/java/src/org/broadinstitute/sting/commandline/ArgumentSource.java @@ -28,6 +28,7 @@ package org.broadinstitute.sting.commandline; import org.broadinstitute.sting.utils.StingException; import java.lang.reflect.Field; +import java.lang.annotation.Annotation; import java.util.Arrays; import java.util.List; @@ -152,6 +153,14 @@ public class ArgumentSource { return typeDescriptor.parse( this, values ); } + /** + * Returns whether this field is required. Note that flag fields are always forced to 'not required'. + * @return True if the field is mandatory and not a boolean flag. False otherwise. + */ + public boolean isRequired() { + return (Boolean)CommandLineUtils.getValue(ArgumentTypeDescriptor.getArgumentAnnotation(this),"required"); + } + /** * Returns true if the argument is a flag (a 0-valued argument). * @return True if argument is a flag; false otherwise. diff --git a/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java b/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java index 4e83918a4..893709a42 100644 --- a/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java +++ b/java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java @@ -43,7 +43,7 @@ import java.io.OutputStream; * @author mhanna * @version 0.1 */ -public abstract class ArgumentTypeDescriptor { +public abstract class ArgumentTypeDescriptor { private static Class[] ARGUMENT_ANNOTATIONS = {Input.class, Output.class, Argument.class}; /** @@ -142,13 +142,19 @@ public abstract class ArgumentTypeDescriptor { * @return The default definition for this argument source. */ protected ArgumentDefinition createDefaultArgumentDefinition( ArgumentSource source ) { - return new ArgumentDefinition( getArgumentAnnotation(source), + Annotation argumentAnnotation = getArgumentAnnotation(source); + return new ArgumentDefinition( ArgumentIOType.getIOType(argumentAnnotation), source.field.getType(), - source.field.getName(), + ArgumentDefinition.getFullName(argumentAnnotation, source.field.getName()), + ArgumentDefinition.getShortName(argumentAnnotation), + ArgumentDefinition.getDoc(argumentAnnotation), + source.isRequired() && !source.overridesDefault() && !source.isFlag(), source.isFlag(), source.isMultiValued(), source.isHidden(), getCollectionComponentType(source.field), + ArgumentDefinition.getExclusiveOf(argumentAnnotation), + ArgumentDefinition.getValidationRegex(argumentAnnotation), getValidOptions(source) ); } @@ -222,7 +228,7 @@ public abstract class ArgumentTypeDescriptor { * @return Argument description annotation associated with the given field. */ @SuppressWarnings("unchecked") - protected Annotation getArgumentAnnotation( ArgumentSource source ) { + protected static Annotation getArgumentAnnotation( ArgumentSource source ) { for (Class annotation: ARGUMENT_ANNOTATIONS) if (source.field.isAnnotationPresent(annotation)) return source.field.getAnnotation(annotation); @@ -463,9 +469,7 @@ class MultiplexArgumentTypeDescriptor extends ArgumentTypeDescriptor { @Override public boolean createsTypeDefault(ArgumentSource source,Class type) { - if(multiplexer == null || multiplexedIds == null) - throw new StingException("No multiplexed ids available"); - // Always create a multiplexed mapping. + // Multiplexing always creates a type default. return true; } diff --git a/java/src/org/broadinstitute/sting/commandline/CommandLineUtils.java b/java/src/org/broadinstitute/sting/commandline/CommandLineUtils.java index 9b5200b43..28045cd9a 100644 --- a/java/src/org/broadinstitute/sting/commandline/CommandLineUtils.java +++ b/java/src/org/broadinstitute/sting/commandline/CommandLineUtils.java @@ -26,10 +26,12 @@ package org.broadinstitute.sting.commandline; import org.broadinstitute.sting.utils.classloader.JVMUtils; +import org.broadinstitute.sting.utils.StingException; import org.broadinstitute.sting.gatk.GenomeAnalysisEngine; import org.broadinstitute.sting.gatk.walkers.Walker; import java.util.*; +import java.lang.annotation.Annotation; /** * Static utility methods for working with command-line arguments. @@ -86,6 +88,19 @@ public class CommandLineUtils { } return sb.toString(); - } + } + /** + * A hack to get around the fact that Java doesn't like inheritance in Annotations. + * @param annotation to run the method on + * @param method the method to invoke + * @return the return value of the method + */ + public static Object getValue(Annotation annotation, String method) { + try { + return annotation.getClass().getMethod(method).invoke(annotation); + } catch (Exception e) { + throw new StingException("Unable to access method " + method + " on annotation " + annotation.getClass(), e); + } + } } diff --git a/java/src/org/broadinstitute/sting/commandline/Input.java b/java/src/org/broadinstitute/sting/commandline/Input.java index 87fb741ea..3e9f607d6 100644 --- a/java/src/org/broadinstitute/sting/commandline/Input.java +++ b/java/src/org/broadinstitute/sting/commandline/Input.java @@ -33,7 +33,7 @@ import java.lang.annotation.*; @Documented @Inherited @Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.FIELD}) +@Target(ElementType.FIELD) public @interface Input { /** * The full name of the command-line argument. Full names should be @@ -58,8 +58,9 @@ public @interface Input { String doc(); /** - * Is this command-line argument required. The application should exit - * printing help if this command-line argument is not specified. + * Is this argument required. If true, the command-line argument system will + * make a best guess for populating this argument based on the type descriptor, + * and will fail if the type can't be populated. * @return True if the argument is required. False otherwise. */ boolean required() default true; diff --git a/java/src/org/broadinstitute/sting/commandline/Output.java b/java/src/org/broadinstitute/sting/commandline/Output.java index 5386a3a94..22565dbf5 100644 --- a/java/src/org/broadinstitute/sting/commandline/Output.java +++ b/java/src/org/broadinstitute/sting/commandline/Output.java @@ -33,7 +33,7 @@ import java.lang.annotation.*; @Documented @Inherited @Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.FIELD}) +@Target(ElementType.FIELD) public @interface Output { /** * The full name of the command-line argument. Full names should be @@ -58,11 +58,12 @@ public @interface Output { String doc() default "An output file presented to the walker. Will overwrite contents if file exists."; /** - * Is this command-line argument required. The application should exit - * printing help if this command-line argument is not specified. + * Is this argument required. If true, the command-line argument system will + * make a best guess for populating this argument based on the type, and will + * fail if the type can't be populated. * @return True if the argument is required. False otherwise. */ - boolean required() default false; + boolean required() default true; /** * Should this command-line argument be exclusive of others. Should be diff --git a/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java b/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java index fa94b61b5..ecb350c15 100755 --- a/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java +++ b/java/src/org/broadinstitute/sting/commandline/ParsingEngine.java @@ -297,7 +297,7 @@ public class ParsingEngine { */ private void loadValueIntoObject( ArgumentSource source, Object instance, ArgumentMatches argumentMatches ) { // Nothing to load - if( argumentMatches.size() == 0 && !source.overridesDefault()) + if( argumentMatches.size() == 0 && !(source.overridesDefault() && source.isRequired())) return; // Target instance into which to inject the value. diff --git a/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java b/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java index c49802eb7..5efdcbc4d 100644 --- a/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java +++ b/java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java @@ -116,14 +116,19 @@ public class SAMFileWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor private ArgumentDefinition createBAMArgumentDefinition(ArgumentSource source) { Annotation annotation = this.getArgumentAnnotation(source); return new ArgumentDefinition( annotation, + ArgumentIOType.getIOType(annotation), source.field.getType(), DEFAULT_ARGUMENT_FULLNAME, DEFAULT_ARGUMENT_SHORTNAME, + ArgumentDefinition.getDoc(annotation), + true, false, source.isMultiValued(), source.isHidden(), - getCollectionComponentType(source.field), - null ); + null, + null, + null, + null); } /** diff --git a/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/UnifiedGenotyper.java b/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/UnifiedGenotyper.java index 9d2170b7c..fb7c72fa2 100755 --- a/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/UnifiedGenotyper.java +++ b/java/src/org/broadinstitute/sting/gatk/walkers/genotyper/UnifiedGenotyper.java @@ -60,10 +60,10 @@ public class UnifiedGenotyper extends LocusWalker annotationsToUse = new ArrayList(); @@ -87,9 +87,6 @@ public class UnifiedGenotyper extends LocusWalker