From dc748d9c9c242d9283e84ddc224e51d8171a514d Mon Sep 17 00:00:00 2001 From: hanna Date: Wed, 20 May 2009 19:01:25 +0000 Subject: [PATCH] Integrate more feedback on command-line argument system. Focus on help formatter: separate required from optional but otherwise keep ordering the same, reorder GATK arguments by usage. git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@764 348d0f76-0448-11de-a6fe-93d51630548a --- .../sting/gatk/CommandLineGATK.java | 4 +- .../sting/gatk/GATKArgumentCollection.java | 95 ++++++----- .../utils/cmdLine/ArgumentDefinitions.java | 12 +- .../sting/utils/cmdLine/HelpFormatter.java | 155 ++++++++---------- .../sting/utils/cmdLine/ParsingEngine.java | 2 +- 5 files changed, 122 insertions(+), 146 deletions(-) diff --git a/java/src/org/broadinstitute/sting/gatk/CommandLineGATK.java b/java/src/org/broadinstitute/sting/gatk/CommandLineGATK.java index e6ceb15b5..5d39465ee 100755 --- a/java/src/org/broadinstitute/sting/gatk/CommandLineGATK.java +++ b/java/src/org/broadinstitute/sting/gatk/CommandLineGATK.java @@ -118,8 +118,8 @@ public class CommandLineGATK extends CommandLineProgram { */ @Override protected Class[] getArgumentSources() { - if (analysisName == null) - throw new IllegalArgumentException("Must provide analysis name"); + // No walker info? No plugins. + if (analysisName == null) return new Class[] {}; walkerManager = new WalkerManager(pluginPathName); diff --git a/java/src/org/broadinstitute/sting/gatk/GATKArgumentCollection.java b/java/src/org/broadinstitute/sting/gatk/GATKArgumentCollection.java index aa0fd53cb..1dce36cc8 100755 --- a/java/src/org/broadinstitute/sting/gatk/GATKArgumentCollection.java +++ b/java/src/org/broadinstitute/sting/gatk/GATKArgumentCollection.java @@ -50,29 +50,28 @@ public class GATKArgumentCollection { public GATKArgumentCollection() { } + @Element(required=false) + public String analysisName = null; + + @ElementMap(entry = "analysis_argument", key = "key", attribute = true, inline = true, required=false) + public Map walkerArgs = new HashMap(); + // parameters and their defaults @ElementList @Argument(fullName = "input_file", shortName = "I", doc = "SAM or BAM file(s)", required = false) public List samFiles = new ArrayList(); @Element(required=false) - @Argument(fullName = "maximum_reads", shortName = "M", doc = "Maximum number of reads to process before exiting", required = false) - public String maximumReads = "-1"; - - @Element(required=false) - @Argument(fullName = "validation_strictness", shortName = "S", doc = "How strict should we be with validation (LENIENT|SILENT|STRICT)", required = false) - public String strictnessLevel = "strict"; + @Argument(fullName = "intervals", shortName = "L", doc = "A list of genomic intervals over which to operate. Can be explicitly specified on the command line or in a file.", required = false) + public String intervals = null; @Element(required=false) @Argument(fullName = "reference_sequence", shortName = "R", doc = "Reference sequence file", required = false) public File referenceFile = null; - @Element(required=false) - public String analysisName = null; - - // parameters and their defaults - @ElementMap(entry = "analysis_argument", key = "key", attribute = true, inline = true, required=false) - public Map walkerArgs = new HashMap(); + @ElementList(required=false) + @Argument(fullName = "rodBind", shortName = "B", doc = "Bindings for reference-ordered data, in the form ,,", required = false) + public ArrayList RODBindings = new ArrayList(); @Element(required=false) @Argument(fullName = "DBSNP", shortName = "D", doc = "DBSNP file", required = false) @@ -86,38 +85,6 @@ public class GATKArgumentCollection { @Argument(fullName = "hapmap_chip", shortName = "hc", doc = "Hapmap chip file", required = false) public String HAPMAPChipFile = null; - @Element(required=false) - @Argument(fullName = "threaded_IO", shortName = "P", doc = "If set, enables threaded I/O operations", required = false) - public Boolean enabledThreadedIO = false; - - @Element(required=false) - @Argument(fullName = "unsafe", shortName = "U", doc = "If set, enables unsafe operations, nothing will be checked at runtime.", required = false) - public Boolean unsafe = false; - - @Element(required=false) - @Argument(fullName = "sort_on_the_fly", shortName = "sort", doc = "Maximum number of reads to sort on the fly", required = false) - public Integer maximumReadSorts = null; - - @Element(required=false) - @Argument(fullName = "downsample_to_fraction", shortName = "dfrac", doc = "Fraction [0.0-1.0] of reads to downsample to", required = false) - public Double downsampleFraction = null; - - @Element(required=false) - @Argument(fullName = "downsample_to_coverage", shortName = "dcov", doc = "Coverage [integer] to downsample to", required = false) - public Integer downsampleCoverage = null; - - @Element(required=false) - @Argument(fullName = "intervals", shortName = "L", doc = "A list of genomic intervals over which to operate. Can be explicitly specified on the command line or in a file.", required = false) - public String intervals = null; - - @Element(required=false) - @Argument(fullName = "all_loci", shortName = "A", doc = "Should we process all loci, not just those covered by reads", required = false) - public Boolean walkAllLoci = false; - - @Element(required=false) - @Argument(fullName = "disablethreading", shortName = "dt", doc = "Disable experimental threading support.", required = false) - public Boolean disableThreading = false; - /** An output file presented to the walker. */ @Element(required=false) @Argument(fullName = "out", shortName = "o", doc = "An output file presented to the walker. Will overwrite contents if file exists.", required = false) @@ -133,15 +100,47 @@ public class GATKArgumentCollection { @Argument(fullName = "outerr", shortName = "oe", doc = "A joint file for 'normal' and error output presented to the walker. Will overwrite contents if file exists.", required = false) public String outErrFileName = null; + @Element(required=false) + @Argument(fullName = "all_loci", shortName = "A", doc = "Should we process all loci, not just those covered by reads", required = false) + public Boolean walkAllLoci = false; + + @Element(required=false) + @Argument(fullName = "maximum_reads", shortName = "M", doc = "Maximum number of reads to process before exiting", required = false) + public String maximumReads = "-1"; + + @Element(required=false) + @Argument(fullName = "sort_on_the_fly", shortName = "sort", doc = "Maximum number of reads to sort on the fly", required = false) + public Integer maximumReadSorts = null; + + @Element(required=false) + @Argument(fullName = "downsample_to_fraction", shortName = "dfrac", doc = "Fraction [0.0-1.0] of reads to downsample to", required = false) + public Double downsampleFraction = null; + + @Element(required=false) + @Argument(fullName = "downsample_to_coverage", shortName = "dcov", doc = "Coverage [integer] to downsample to", required = false) + public Integer downsampleCoverage = null; + + @Element(required=false) + @Argument(fullName = "validation_strictness", shortName = "S", doc = "How strict should we be with validation (LENIENT|SILENT|STRICT)", required = false) + public String strictnessLevel = "strict"; + + @Element(required=false) + @Argument(fullName = "unsafe", shortName = "U", doc = "If set, enables unsafe operations, nothing will be checked at runtime.", required = false) + public Boolean unsafe = false; + + @Element(required=false) + @Argument(fullName = "threaded_IO", shortName = "P", doc = "If set, enables threaded I/O operations", required = false) + public Boolean enabledThreadedIO = false; + + @Element(required=false) + @Argument(fullName = "disablethreading", shortName = "dt", doc = "Disable experimental threading support.", required = false) + public Boolean disableThreading = false; + /** How many threads should be allocated to this analysis. */ @Element(required=false) @Argument(fullName = "numthreads", shortName = "nt", doc = "How many threads should be allocated to running this analysis.", required = false) public int numberOfThreads = 1; - @ElementList(required=false) - @Argument(fullName = "rodBind", shortName = "B", doc = "", required = false) - public ArrayList RODBindings = new ArrayList(); - /** * marshal the data out to a object * diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java index f336fbcfc..4df99dd68 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java @@ -62,7 +62,7 @@ class ArgumentDefinitions implements Iterable { while( definitionGroupIterator.hasNext() ) { ArgumentDefinitionGroup candidate = definitionGroupIterator.next(); if( candidate.groupNameMatches(argumentDefinitionGroup) ) { - argumentDefinitionGroup = argumentDefinitionGroup.mergeInto(candidate); + argumentDefinitionGroup = candidate.merge(argumentDefinitionGroup); definitionGroupIterator.remove(); } } @@ -184,11 +184,11 @@ class ArgumentDefinitionGroup implements Iterable { /** * The argument definitions associated with this group. */ - public final Collection argumentDefinitions; + public final List argumentDefinitions; - public ArgumentDefinitionGroup( String groupName, Collection argumentDefinitions ) { + public ArgumentDefinitionGroup( String groupName, List argumentDefinitions ) { this.groupName = groupName; - this.argumentDefinitions = Collections.unmodifiableCollection( argumentDefinitions ); + this.argumentDefinitions = Collections.unmodifiableList( argumentDefinitions ); } /** @@ -207,12 +207,12 @@ class ArgumentDefinitionGroup implements Iterable { * group since argument groups are supposed to be immutable. Asserts that * both argument groups have the same name. */ - public ArgumentDefinitionGroup mergeInto( ArgumentDefinitionGroup other ) { + public ArgumentDefinitionGroup merge( ArgumentDefinitionGroup other ) { if( !groupNameMatches(other) ) throw new StingException("Unable to merge two argument groups with differing names."); // Create a merged definition group. - Collection mergedDefinitions = new ArrayList(); + List mergedDefinitions = new ArrayList(); mergedDefinitions.addAll(this.argumentDefinitions); mergedDefinitions.addAll(other.argumentDefinitions); diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/HelpFormatter.java b/java/src/org/broadinstitute/sting/utils/cmdLine/HelpFormatter.java index d42d13ff3..9458208f0 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/HelpFormatter.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/HelpFormatter.java @@ -1,17 +1,12 @@ package org.broadinstitute.sting.utils.cmdLine; -import org.broadinstitute.sting.utils.StingException; - import java.util.Formatter; import java.util.List; import java.util.ArrayList; import java.util.Iterator; -import java.util.Map; -import java.util.HashMap; import java.util.Comparator; -import java.util.TreeSet; -import java.util.SortedSet; import java.util.Collection; +import java.util.Collections; import java.util.regex.Pattern; import java.util.regex.Matcher; /** @@ -43,80 +38,36 @@ public class HelpFormatter { * @param argumentDefinitions Argument definitions for which help should be printed. */ public void printHelp( String runningInstructions, ArgumentDefinitions argumentDefinitions ) { - SortedSet mainArguments = getMainArguments( argumentDefinitions ); - Map> pluginsByGroup = getPluginArguments( argumentDefinitions ); - - System.out.printf("%s%s%n", - getSynopsis(runningInstructions,mainArguments,pluginsByGroup), - getDetailed(mainArguments,pluginsByGroup) ); - } - - /** - * Retrieve a sorted set of the command-line arguments for the main application from the ArgumentDefinitions object. - * @param argumentDefinitions Predefined argument definitions. - * @return A list of argument definitions. - */ - private SortedSet getMainArguments( ArgumentDefinitions argumentDefinitions ) { - for( ArgumentDefinitionGroup argumentGroup: argumentDefinitions.getArgumentDefinitionGroups() ) { - // A null groupName is the signal that these are the application's main arguments. - if( argumentGroup.groupName == null ){ - SortedSet sortedDefinitions = new TreeSet( new ArgumentDefinitionComparator() ); - sortedDefinitions.addAll( argumentGroup.argumentDefinitions ); - return sortedDefinitions; - } - } - throw new StingException( "Unable to retrieve main command-line arguments." ); - } - - /** - * Retrieve a sorted set of the command-line arguments for application plugin objects. - * @param argumentDefinitions Predefined argument definitions. - * @return A map of argument group name -> a list of argument definitions. - */ - private Map> getPluginArguments( ArgumentDefinitions argumentDefinitions ) { - Map> pluginsByGroup = new HashMap>(); - for( ArgumentDefinitionGroup argumentGroup: argumentDefinitions.getArgumentDefinitionGroups() ) { - // A null groupName is the signal that these are the application's main arguments. - if( argumentGroup.groupName == null ) - continue; - SortedSet sortedDefinitions = new TreeSet( new ArgumentDefinitionComparator() ); - sortedDefinitions.addAll( argumentGroup.argumentDefinitions ); - pluginsByGroup.put( argumentGroup.groupName, sortedDefinitions ); - } - return pluginsByGroup; + List argumentGroups = prepareArgumentGroups( argumentDefinitions ); + System.out.printf("%s%s%n",getSynopsis(runningInstructions,argumentGroups),getDetailed(argumentGroups) ); } /** * Gets the synopsis: the actual command to run. - * @param mainArguments Main program arguments. * @param runningInstructions Instructions on how to run hte application. - * @param pluginArgumentGroups Groups of plugin arguments + * @param argumentGroups Program arguments sorted in order of definition group displays. * @return A synopsis line. */ private String getSynopsis( String runningInstructions, - SortedSet mainArguments, - Map> pluginArgumentGroups ) { + List argumentGroups ) { // Build out the synopsis all as one long line. StringBuilder lineBuilder = new StringBuilder(); Formatter lineFormatter = new Formatter( lineBuilder ); lineFormatter.format("java %s", runningInstructions); - List argumentDefinitions = new ArrayList(); - argumentDefinitions.addAll( mainArguments ); - for( SortedSet pluginArguments: pluginArgumentGroups.values() ) - argumentDefinitions.addAll( pluginArguments ); - - for( ArgumentDefinition argumentDefinition: argumentDefinitions ) { - lineFormatter.format(" "); - if( !argumentDefinition.required ) lineFormatter.format("["); - if( argumentDefinition.shortName != null ) - lineFormatter.format("-%s", argumentDefinition.shortName); - else - lineFormatter.format("--%s", argumentDefinition.fullName); - if( !argumentDefinition.isFlag() ) - lineFormatter.format(" <%s>", argumentDefinition.fullName); - if( !argumentDefinition.required ) lineFormatter.format("]"); + for( ArgumentDefinitionGroup argumentGroup: argumentGroups ) { + for( ArgumentDefinition argumentDefinition: argumentGroup.argumentDefinitions ) { + lineFormatter.format(" "); + if( !argumentDefinition.required ) lineFormatter.format("["); + if( argumentDefinition.shortName != null ) + lineFormatter.format("-%s", argumentDefinition.shortName); + else + lineFormatter.format("--%s", argumentDefinition.fullName); + if( !argumentDefinition.isFlag() ) + lineFormatter.format(" <%s>", argumentDefinition.fullName); + if( !argumentDefinition.required ) lineFormatter.format("]"); + } } // Word wrap the synopsis. @@ -137,23 +88,27 @@ public class HelpFormatter { /** * Gets detailed output about each argument type. - * @param mainArguments Main program arguments. - * @param pluginArgumentGroups Groups of plugin arguments + * @param argumentGroups Collection of program arguments sorted according to how they should be shown. * @return Detailed text about all arguments. */ - private String getDetailed( SortedSet mainArguments, Map> pluginArgumentGroups ) { + private String getDetailed( List argumentGroups ) { StringBuilder builder = new StringBuilder(); - builder.append( getDetailForGroup( mainArguments ) ); - - for( String groupName: pluginArgumentGroups.keySet() ) { - builder.append( String.format("Arguments for %s:%n", groupName ) ); - builder.append( getDetailForGroup( pluginArgumentGroups.get(groupName) ) ); + for( ArgumentDefinitionGroup argumentGroup: argumentGroups ) { + if( argumentGroup.groupName != null && argumentGroup.argumentDefinitions.size() != 0 ) + builder.append( String.format("%nArguments for %s:%n", argumentGroup.groupName ) ); + builder.append( getDetailForGroup( argumentGroup.argumentDefinitions ) ); } + return builder.toString(); } - private String getDetailForGroup( SortedSet argumentDefinitions ) { + /** + * Gets a detailed description for a given argument group. + * @param argumentDefinitions The argument definitions contained withina group. + * @return A string giving detailed info about the contents of this group. + */ + private String getDetailForGroup( List argumentDefinitions ) { StringBuilder builder = new StringBuilder(); Formatter formatter = new Formatter( builder ); @@ -237,21 +192,43 @@ public class HelpFormatter { } /** - * A comparator to reorder arguments in alphabetical order. + * Extract the argument definition groups from the argument definitions and arrange them appropriately. + * For help, we want the arguments sorted as they are declared in the class. However, required arguments + * should appear before optional arguments. + * @param argumentDefinitions Argument definitions from which to extract argument groups. + * @return A list of argument groups sorted in display order. */ - private class ArgumentDefinitionComparator implements Comparator { - public int compare( ArgumentDefinition lhs, ArgumentDefinition rhs ) { - if( lhs.shortName == null && rhs.shortName == null ) - return lhs.fullName.compareTo( rhs.fullName ); - // short names always come before long names. - else if ( lhs.shortName == null ) - return -1; - else if ( rhs.shortName == null ) - return 1; - else if ( lhs.shortName.equals(rhs.shortName) ) - return lhs.fullName.compareTo( rhs.fullName ); - else - return lhs.shortName.compareTo( rhs.shortName ); + private List prepareArgumentGroups( ArgumentDefinitions argumentDefinitions ) { + // Sort the list of argument definitions according to how they should be shown. + // Put the sorted results into a new cloned data structure. + Comparator definitionComparator = new Comparator() { + public int compare( ArgumentDefinition lhs, ArgumentDefinition rhs ) { + if( lhs.required && rhs.required ) return 0; + if( lhs.required ) return -1; + if( rhs.required ) return 1; + return 0; + } + }; + + List argumentGroups = new ArrayList(); + for( ArgumentDefinitionGroup argumentGroup: argumentDefinitions.getArgumentDefinitionGroups() ) { + List sortedDefinitions = new ArrayList( argumentGroup.argumentDefinitions ); + Collections.sort( sortedDefinitions, definitionComparator ); + argumentGroups.add( new ArgumentDefinitionGroup(argumentGroup.groupName,sortedDefinitions) ); } + + // Sort the argument groups themselves with main arguments first, followed by plugins sorted in name order. + Comparator groupComparator = new Comparator() { + public int compare( ArgumentDefinitionGroup lhs, ArgumentDefinitionGroup rhs ) { + if( lhs.groupName == null && rhs.groupName == null ) return 0; + if( lhs.groupName == null ) return -1; + if( rhs.groupName == null ) return 1; + return lhs.groupName.compareTo(rhs.groupName); + } + }; + Collections.sort( argumentGroups, groupComparator ); + + + return argumentGroups; } } diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java index d64f28ec3..bf6e51b8d 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java @@ -79,7 +79,7 @@ public class ParsingEngine { * @param source An argument source from which to extract command-line arguments. */ public void addArgumentSource( String sourceName, Class source ) { - Collection argumentsFromSource = new ArrayList(); + List argumentsFromSource = new ArrayList(); while( source != null ) { Field[] fields = source.getDeclaredFields(); for( Field field: fields ) {