Prototype, buggy implementation of walker command-line arguments. Doesn't

(yet) deal elegantly with even simple cases.


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

build.xml

@@ -110,6 +110,7 @@
         <!-- Compile the java code from ${src} into build -->
         <javac destdir="build" classpathref="thirdparty.dependencies"
                debug="true" debuglevel="lines,vars,source">
+            <!--compilerarg value="-Xlint:unchecked" /-->
             <src refid="source"/>
         </javac>
     </target>
@@ -196,7 +197,7 @@
         </jar>
     </target>
 
-    <target name="javadoc">
+    <target name="javadoc" description="generates javadoc">
         <mkdir dir="javadoc"/>
         <javadoc sourcepathref="${target}.srcdir" destdir="javadoc"/>
     </target>

ivy.xml

@@ -7,6 +7,6 @@
     <dependency org="junit" name="junit" rev="4.4" />
     <dependency org="log4j" name="log4j" rev="1.2.15" />
     <dependency org="colt" name="colt" rev="1.2.0" />
-    <dependency org="commons-cli" name="commons-cli" rev="1.1" />
+    <dependency org="commons-cli" name="commons-cli" rev="1.2" />
   </dependencies>
 </ivy-module>

java/src/org/broadinstitute/sting/gatk/GenomeAnalysisTK.java

@@ -12,6 +12,7 @@ import org.broadinstitute.sting.gatk.refdata.rodDbSNP;
 import org.broadinstitute.sting.gatk.refdata.rodGFF;
 import org.broadinstitute.sting.gatk.walkers.LocusWalker;
 import org.broadinstitute.sting.gatk.walkers.ReadWalker;
+import org.broadinstitute.sting.gatk.walkers.Walker;
 import org.broadinstitute.sting.utils.FastaSequenceFile2;
 import org.broadinstitute.sting.utils.GenomeLoc;
 import org.broadinstitute.sting.utils.Utils;
@@ -42,6 +43,7 @@ public class GenomeAnalysisTK extends CommandLineProgram {
 
     // our walker manager
     private WalkerManager walkerManager = null;
+    private Walker my_walker = null;
 
     public String pluginPathName = null;
     private TraversalEngine engine = null;
@@ -60,12 +62,12 @@ public class GenomeAnalysisTK extends CommandLineProgram {
      * Flags don't take an argument, the associated Boolean gets set to true if the flag appears on the command line.
      */
     protected void setupArgs() {
-        m_parser.addRequiredlArg("input_file", "I", "SAM or BAM file for validation", "INPUT_FILE");
+        m_parser.addRequiredArg("input_file", "I", "SAM or BAM file for validation", "INPUT_FILE");
         m_parser.addOptionalArg("maximum_reads", "M", "Maximum number of reads to process before exiting", "MAX_READS_ARG");
         m_parser.addOptionalArg("validation_strictness", "S", "How strict should we be with validation", "STRICTNESS_ARG");
         m_parser.addOptionalArg("reference_sequence", "R", "Reference sequence file", "REF_FILE_ARG");
         m_parser.addOptionalArg("genome_region", "L", "Genome region to operation on: from chr:start-end", "REGION_STR");
-        m_parser.addRequiredlArg("analysis_type", "T", "Type of analysis to run", "Analysis_Name");
+        m_parser.addRequiredArg("analysis_type", "T", "Type of analysis to run", "Analysis_Name");
         m_parser.addOptionalArg("DBSNP", "D", "DBSNP file", "DBSNP_FILE");
         m_parser.addOptionalArg("Hapmap", "H", "Hapmap file", "HAPMAP_FILE");
         m_parser.addOptionalFlag("threaded_IO", "P", "If set, enables threaded I/O operations", "ENABLED_THREADED_IO");
@@ -74,6 +76,32 @@ public class GenomeAnalysisTK extends CommandLineProgram {
         m_parser.addOptionalArg("intervals_file", "V", "File containing list of genomic intervals to operate on. line := <contig> <start> <end>", "INTERVALS_FILE");
     }
 
+    /**
+     * GATK can add arguments dynamically based on analysis type.
+     * @return true
+     */
+    @Override
+    protected boolean canAddArgumentsDynamically() { return true; }
+
+    /**
+     * GATK provides the walker as an argument source.  As a side-effect, initializes the walker variable.
+     * @return List of walkers to load dynamically.
+     */
+    @Override
+    protected Object[] getArgumentSources() {
+        if( Analysis_Name == null )
+            throw new IllegalArgumentException("Must provide analysis name");
+
+        walkerManager = new WalkerManager( pluginPathName );
+
+        if( !walkerManager.doesWalkerExist(Analysis_Name) )
+            throw new IllegalArgumentException("Invalid analysis name");
+
+        my_walker = walkerManager.getWalkerByName(Analysis_Name);
+
+        return new Object[] { my_walker };
+    }
+
     /**
      * Required main method implementation.
      */
@@ -82,10 +110,6 @@ public class GenomeAnalysisTK extends CommandLineProgram {
     }
 
     protected int execute() {
-
-
-        walkerManager = new WalkerManager(pluginPathName);
-
         final boolean TEST_ROD = false;
         List<ReferenceOrderedData> rods = new ArrayList<ReferenceOrderedData>();
 
@@ -155,15 +179,10 @@ public class GenomeAnalysisTK extends CommandLineProgram {
 
         //LocusWalker<Integer,Integer> walker = new PileupWalker();
 
-        // Try to get the walker specified
+        if( my_walker == null )
-        Object my_walker;
+            throw new RuntimeException( "Sanity check failed -- no walker present." );
-        if (walkerManager.doesWalkerExist(Analysis_Name)) {
-            my_walker = walkerManager.getWalkerByName(Analysis_Name);
-        } else {
-            logger.fatal("Could not find walker " + Analysis_Name);
-            return 0;
-        }
 
+        // Try to get the walker specified
         try {
             LocusWalker<?, ?> walker = (LocusWalker<?, ?>) my_walker;
             if ( INTERVALS_FILE == null )

java/src/org/broadinstitute/sting/gatk/WalkerManager.java

@@ -5,6 +5,7 @@ import net.sf.functionalj.reflect.JdkStdReflect;
 import net.sf.functionalj.FunctionN;
 import net.sf.functionalj.Functions;
 
+import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.io.File;
 import java.io.FilenameFilter;
@@ -20,6 +21,7 @@ import java.util.jar.JarEntry;
 import java.util.jar.JarInputStream;
 
 import org.broadinstitute.sting.gatk.walkers.Walker;
+import org.broadinstitute.sting.utils.cmdLine.Argument;
 
 /**
  * Created by IntelliJ IDEA.
@@ -88,7 +90,8 @@ public class WalkerManager {
      * @return The walker object if found; null otherwise.
      */
     public Walker getWalkerByName(String walkerName) {
-        return walkers.get(walkerName);
+        Walker walker = walkers.get(walkerName);
+        return walker;
     }
 
     /**

java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java

@@ -0,0 +1,26 @@
+package org.broadinstitute.sting.utils.cmdLine;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Created by IntelliJ IDEA.
+ * User: hanna
+ * Date: Mar 24, 2009
+ * Time: 11:11:36 AM
+ * To change this template use File | Settings | File Templates.
+ */
+@Documented
+@Inherited
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.FIELD})
+public @interface Argument {
+    String fullName() default "";
+    String shortName() default "";
+    String doc() default "";
+    boolean required() default true;
+}

java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentParser.java

@@ -11,6 +11,8 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 
+import org.broadinstitute.sting.utils.Pair;
+
 /**
  * User: aaron
  * Date: Mar 19, 2009
@@ -33,7 +35,7 @@ public class ArgumentParser {
     private ArrayList<String> m_option_names = new ArrayList<String>();
 
     // where we eventually want the values to land
-    private HashMap<String, Field> m_storageLocations = new HashMap<String, Field>();
+    private HashMap<String, Pair<Object,Field>> m_storageLocations = new HashMap<String, Pair<Object,Field>>();
 
     // create Options object
     protected Options m_options = new Options();
@@ -100,7 +102,18 @@ public class ArgumentParser {
      * @param opt        the option
      */
     private void AddToOptionStorage(String name, String letterform, String fieldname, Option opt) {
+        AddToOptionStorage(name, letterform, getField(prog, fieldname), opt);
+    }
 
+    /**
+     * Used locally to add to the options storage we have, for latter processing
+     *
+     * @param name       the name of the option
+     * @param letterform it's short form
+     * @param field      what field it should be stuck into on the calling class
+     * @param opt        the option
+     */
+    private void AddToOptionStorage(String name, String letterform, Pair<Object,Field> field, Option opt) {
         // add to the option list
         m_options.addOption(opt);
 
@@ -110,20 +123,23 @@ public class ArgumentParser {
         }
 
         // add the object with it's name to the storage location
-        try {
+        m_storageLocations.put( name, field );
-            m_storageLocations.put(name, prog.getClass().getField(fieldname));
-        } catch (NoSuchFieldException e) {
-            logger.fatal("Failed to find the field specified by the fieldname parameter.");
-            throw new RuntimeException(e.getMessage());
-        }
 
         // add to the list of m_options
         m_option_names.add(letterform);
     }
 
-
+    private Pair<Object,Field> getField( Object obj, String fieldName ) {
+        try {
+            return new Pair<Object,Field>( obj, obj.getClass().getField(fieldName) );
+        } catch (NoSuchFieldException e) {
+            logger.fatal("Failed to find the field specified by the fieldname parameter.");
+            throw new RuntimeException(e.getMessage());
+        }
+    }
+    
     /**
-     * addRequiredlArg
+     * addRequiredArg
      * <p/>
      * Adds a required argument to check on the command line
      *
@@ -132,7 +148,7 @@ public class ArgumentParser {
      * @param description the description of the argument
      * @param fieldname   what field it should be stuck into on the calling class
      */
-    public void addRequiredlArg(String name, String letterform, String description, String fieldname) {
+    public void addRequiredArg(String name, String letterform, String description, String fieldname) {
         // we always want the help option to be available
         Option opt = OptionBuilder.isRequired()
                 .withLongOpt(name)
@@ -169,7 +185,7 @@ public class ArgumentParser {
 
 
     /**
-     * addRequiredlArg
+     * addRequiredArg
      * <p/>
      * Adds a required argument to check on the command line
      *
@@ -178,7 +194,7 @@ public class ArgumentParser {
      * @param description the description of the argument
      * @param fieldname   what field it should be stuck into on the calling class
      */
-    public void addRequiredlArgList(String name, String letterform, String description, String fieldname) {
+    public void addRequiredArgList(String name, String letterform, String description, String fieldname) {
 
         // we always want the help option to be available
         Option opt = OptionBuilder.isRequired()
@@ -226,7 +242,7 @@ public class ArgumentParser {
 
 
     /**
-     * addRequiredlFlag
+     * addRequiredFlag
      * <p/>
      * Adds a required argument to check on the command line
      *
@@ -235,7 +251,7 @@ public class ArgumentParser {
      * @param description the description of the argument
      * @param fieldname   what field it should be stuck into on the calling class
      */
-    public void addRequiredlFlag(String name, String letterform, String description, String fieldname) {
+    public void addRequiredFlag(String name, String letterform, String description, String fieldname) {
 
         // if they've passed a non-Boolean as a object, beat them
         try {
@@ -254,7 +270,6 @@ public class ArgumentParser {
 
         // add it to the option
         AddToOptionStorage(name, letterform, fieldname, opt);
-
     }
 
 
@@ -265,45 +280,87 @@ public class ArgumentParser {
      *
      * @param args the command line arguments we recieved
      */
-    public void processArgs(String[] args) throws ParseException {
+    public void processArgs(String[] args, boolean allowUnrecognized) throws ParseException {
-        CommandLineParser parser = new PosixParser();
+        OurPosixParser parser = new OurPosixParser();
+        Collection<Option> opts = m_options.getOptions();
 
         try {
-            Collection<Option> opts = m_options.getOptions();
+            parser.parse(m_options, args, !allowUnrecognized);
+        }
+        catch (UnrecognizedOptionException e) {
+            // we don't care about unknown exceptions right now
+            logger.warn(e.getMessage());
+            if(!allowUnrecognized)
+                throw e;
+        }
 
-            CommandLine cmd = parser.parse(m_options, args);
+        // Apache CLI can ignore unrecognized arguments with a boolean flag, but
+        // you can't get to the unparsed args.  Override PosixParser with a class
+        // that can reach in and extract the protected command line.
+        // TODO: Holy crap this is wacky.  Find a cleaner way.
+        CommandLine cmd = parser.getCmd();
 
-            // logger.info("We have " + opts.size() + " options");
+        // logger.info("We have " + opts.size() + " options");
-            for (Option opt : opts) {
+        for (Option opt : opts) {
-                if (cmd.hasOption(opt.getOpt())) {
+            if (cmd.hasOption(opt.getOpt())) {
-                    if (opt.hasArg()) {
+                if (opt.hasArg()) {
-                        //logger.info("looking at " + m_storageLocations.get(opt.getLongOpt()));
+                    //logger.info("looking at " + m_storageLocations.get(opt.getLongOpt()));
-                        Field f = m_storageLocations.get(opt.getLongOpt());
+                    Object obj = m_storageLocations.get(opt.getLongOpt()).first;
-                        try {
+                    Field field = m_storageLocations.get(opt.getLongOpt()).second;
-                            f.set(prog, constructFromString(f, cmd.getOptionValue(opt.getOpt())));
-                        } catch (IllegalAccessException e) {
-                            logger.fatal("processArgs: cannot convert field " + f.toString());
-                            throw new RuntimeException("processArgs: Failed conversion " + e.getMessage());
-                        }
-                    } else {
 
-                        Field f = m_storageLocations.get(opt.getLongOpt());
+                    try {
-                        try {
+                        field.set(obj, constructFromString(field, cmd.getOptionValue(opt.getOpt())));
-                            //logger.fatal("about to parse field " + f.getName());
+                    } catch (IllegalAccessException e) {
-                            f.set(prog, new Boolean(true));
+                        logger.fatal("processArgs: cannot convert field " + field.toString());
-                        } catch (IllegalAccessException e) {
+                        throw new RuntimeException("processArgs: Failed conversion " + e.getMessage());
-                            logger.fatal("processArgs: cannot convert field " + f.toString());
+                    }
-                            throw new RuntimeException("processArgs: Failed conversion " + e.getMessage());
+                } else {
-                        }
+                    Object obj = m_storageLocations.get(opt.getLongOpt()).first;
+                    Field field = m_storageLocations.get(opt.getLongOpt()).second;
+
+                    try {
+                        //logger.fatal("about to parse field " + f.getName());
+                        field.set(obj, new Boolean(true));
+                    } catch (IllegalAccessException e) {
+                        logger.fatal("processArgs: cannot convert field " + field.toString());
+                        throw new RuntimeException("processArgs: Failed conversion " + e.getMessage());
                     }
                 }
             }
-        } catch (UnrecognizedOptionException e) {
-            // we don't care about unknown exceptions right now
-            logger.warn(e.getMessage());
         }
     }
 
+    private class OurPosixParser extends PosixParser {
+        public CommandLine getCmd() { return cmd; }
+    }
+
+    /**
+     * Extract arguments stored in annotations from fields of a given class.
+     * @param source
+     */
+    public void addArgumentSource( Object source ) {
+        Field[] fields = source.getClass().getFields();
+        for(Field field: fields) {
+            Argument arg = field.getAnnotation(Argument.class);
+            if(arg == null)
+                continue;
+
+            String fullName = (arg.fullName().length() != 0) ? arg.fullName() : field.getName().trim().toLowerCase();
+            String shortName = (arg.shortName().length() != 0) ? arg.shortName() : fullName.substring(0,1);
+            if(shortName.length() != 1)
+                throw new IllegalArgumentException("Invalid short name: " + shortName);
+            String description = arg.required() ? "(Required Flag) " + arg.doc() : arg.doc();
+
+            // TODO: Handle flags, handle lists
+            OptionBuilder ob = OptionBuilder.withLongOpt(fullName).withArgName(fullName).hasArg();
+            if( arg.required() ) ob = ob.isRequired();
+            if( description.length() != 0 ) ob = ob.withDescription( description );
+
+            Option option = ob.create( shortName );
+
+            AddToOptionStorage(fullName, shortName, new Pair<Object,Field>( source, field ), option );
+        }
+    }
 
     private Object constructFromString(Field f, String str) {
         Type type = f.getType();
@@ -352,8 +409,8 @@ public class ArgumentParser {
  public static void main(String[] args) {
  ArgumentParser p = new ArgumentParser("CrapApp");
  p.setupDefaultArgs();
- p.addRequiredlArg("Flag","F","a required arg");
+ p.addRequiredArg("Flag","F","a required arg");
- p.addRequiredlFlag("Sub","S","a required flag");
+ p.addRequiredFlag("Sub","S","a required flag");
  p.addOptionalArg("Boat","T","Maybe you want a boat?");
  String[] str = {"--Flag","rrr","-T","ppp", "--Flag","ttt"};
  p.processArgs(str);
@@ -368,4 +425,4 @@ public class ArgumentParser {
  }
  }
  }
- */
+ */

java/src/org/broadinstitute/sting/utils/cmdLine/CommandLineProgram.java

@@ -69,6 +69,20 @@ public abstract class CommandLineProgram {
      */
     protected abstract void setupArgs();
 
+    /**
+     * Will this application want to vary its argument list dynamically?
+     * If so, parse the command-line options and then prompt the subclass to return
+     * a list of argument providers.
+     * @return Whether the application should vary command-line arguments dynamically.
+     */
+    protected boolean canAddArgumentsDynamically() { return false; }
+
+    /**
+     * Provide a list of object to inspect, looking for additional command-line arguments.
+     * @return A list of objects to inspect.
+     */
+    protected Object[] getArgumentSources() { return new Object[] {}; }
+
     /**
      * this is the function that the inheriting class can expect to have called
      * when all the argument processing is done
@@ -111,8 +125,17 @@ public abstract class CommandLineProgram {
             clp.setupArgs();
 
             // process the args
-            clp.m_parser.processArgs(args);
+            if( clp.canAddArgumentsDynamically() ) {
-
+                // if the command-line program can toss in extra args, fetch them and reparse the arguments.
+                clp.m_parser.processArgs(args, true);
+                Object[] argumentSources = clp.getArgumentSources();
+                for( Object argumentSource: argumentSources )
+                    clp.addArgumentSource( argumentSource );
+                clp.m_parser.processArgs(args, false);
+            }
+            else {
+                clp.m_parser.processArgs(args, false);
+            }
 
             // if we're in debug mode, set the mode up
             if (clp.debugMode) {
@@ -164,13 +187,17 @@ public abstract class CommandLineProgram {
             // return the result
             System.exit(result);
         }
+        catch (org.apache.commons.cli.ParseException e) {
+            logger.fatal("Unable to pass command line arguments: " + e.getMessage() );
+            clp.m_parser.printHelp();
+        }
         catch (Exception e) {
             // we catch all exceptions here. if it makes it to this level, we're in trouble.  Let's bail!
             // TODO: what if the logger is the exception? hmm...
             logger.fatal("Exception caught by base Command Line Program, with message: " + e.getMessage());
             logger.fatal("with cause: " + e.getCause());
             e.printStackTrace();
-            throw new RuntimeException(e.getMessage());
+            throw new RuntimeException(e);
         }
     }
 
@@ -229,6 +256,14 @@ public abstract class CommandLineProgram {
         logger.setLevel(par);
     }
 
+    /**
+     * Pass along a new set of valid command line arguments.  In this case,
+     * probably a class with @argument or @flag annotations.
+     * @param source
+     */
+    private void addArgumentSource( Object source ) {
+        m_parser.addArgumentSource(source);
+    }
 
     /**
      * we have some default options that should always get checked for in the