Merge pull request #1352 from broadinstitute/mf_othersrequired

Adds othersRequired so that some arguments can require other arguments in order to work.
This commit is contained in:
Mark Fleharty 2016-05-18 15:14:59 -04:00
commit ff532ed0cd
9 changed files with 156 additions and 24 deletions

View File

@ -53,6 +53,7 @@ public abstract class ArgumentDefinitionField extends ArgumentField {
@Override protected String getShortName() { return escape(argumentDefinition.shortName); }
@Override protected boolean isRequired() { return argumentDefinition.required; }
@Override protected String getExclusiveOf() { return escape(argumentDefinition.exclusiveOf); }
@Override protected String getOtherArgumentRequired() { return escape(argumentDefinition.otherArgumentRequired); }
@Override protected String getValidation() { return escape(argumentDefinition.validation); }
protected boolean isFlag() { return argumentDefinition.isFlag; }
protected boolean isMultiValued() { return argumentDefinition.isMultiValued; }
@ -250,6 +251,15 @@ public abstract class ArgumentDefinitionField extends ArgumentField {
exclusiveOf.append(escape(argumentDefinition.fullName)).append("String");
return exclusiveOf.toString();
}
@Override
protected String getOtherArgumentRequired() {
StringBuilder otherArgumentRequired = new StringBuilder(super.getOtherArgumentRequired());
if (otherArgumentRequired.length() > 0)
otherArgumentRequired.append(",");
otherArgumentRequired.append(escape(argumentDefinition.fullName)).append("String");
return otherArgumentRequired.toString();
}
}
// if (intervalFields.contains(argumentDefinition.fullName) && argumentDefinition.ioType == ArgumentIOType.INPUT)
@ -277,6 +287,15 @@ public abstract class ArgumentDefinitionField extends ArgumentField {
exclusiveOf.append(escape(argumentDefinition.fullName));
return exclusiveOf.toString();
}
@Override
protected String getOtherArgumentRequired() {
StringBuilder otherArgumentRequired = new StringBuilder(super.getOtherArgumentRequired());
if (otherArgumentRequired.length() > 0)
otherArgumentRequired.append(",");
otherArgumentRequired.append(escape(argumentDefinition.fullName));
return otherArgumentRequired.toString();
}
}
// if (argumentDefinition.ioType == ArgumentIOType.INPUT)

View File

@ -56,7 +56,8 @@ public abstract class ArgumentField {
public final String getArgumentAddition() {
return String.format("%n" +
"/** %s */%n" +
"@%s(fullName=\"%s\", shortName=\"%s\", doc=\"%s\", required=%s, exclusiveOf=\"%s\", validation=\"%s\")%n" +
"@%s(fullName=\"%s\", shortName=\"%s\", doc=\"%s\", required=%s, exclusiveOf=\"%s\", " +
"otherArgumentRequired=\"%s\", validation=\"%s\")%n" +
"%s%svar %s: %s = %s%n" +
"%s",
getDoc(),
@ -66,6 +67,7 @@ public abstract class ArgumentField {
getDoc(),
isRequired(),
getExclusiveOf(),
getOtherArgumentRequired(),
getValidation(),
getGatherAnnotation(), getPrivacy(), getFieldName(), getFieldType(), getDefaultValue(),
getDefineAddition());
@ -97,6 +99,9 @@ public abstract class ArgumentField {
/** @return A comma separated list of arguments that may be substituted for this field. */
protected String getExclusiveOf() { return ""; }
/** @return A string containing an argument that is necessary for the current argument to work. */
protected String getOtherArgumentRequired() { return ""; }
/** @return A validation string for the argument. */
protected String getValidation() { return ""; }

View File

@ -81,6 +81,15 @@ public @interface Argument {
*/
String exclusiveOf() default "";
/**
* Does this command-line argument require other arguments to go with it?
* Should be a string containing the name of the required argument.
* This option only supports a single required argument.
* @return A string with the other argument that this
* argument should require in order to work.
*/
String otherArgumentRequired() default "";
/**
* Provide a regexp-based validation string.
* @return Non-empty regexp for validation, blank otherwise.

View File

@ -96,6 +96,11 @@ public class ArgumentDefinition {
*/
public final String exclusiveOf;
/**
* Does this argument require another argument?
*/
public final String otherArgumentRequired;
/**
* Can we validate this regular expression?
*/
@ -119,6 +124,7 @@ public class ArgumentDefinition {
* @param isHidden Whether or not this argument should be hidden from the command-line argument system.
* @param componentType For multivalued arguments the type of the components.
* @param exclusiveOf Whether this command line argument is mutually exclusive of other arguments.
* @param otherArgumentRequired Other argument the current argument requires in order to work.
* @param validation A regular expression for command-line argument validation.
* @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.
*/
@ -133,6 +139,7 @@ public class ArgumentDefinition {
boolean isHidden,
Class componentType,
String exclusiveOf,
String otherArgumentRequired,
String validation,
List<String> validOptions) {
this.ioType = ioType;
@ -146,6 +153,7 @@ public class ArgumentDefinition {
this.isHidden = isHidden;
this.componentType = componentType;
this.exclusiveOf = exclusiveOf;
this.otherArgumentRequired = otherArgumentRequired;
this.validation = validation;
this.validOptions = validOptions;
@ -177,6 +185,7 @@ public class ArgumentDefinition {
boolean isHidden,
Class componentType,
String exclusiveOf,
String otherArgumentRequired,
String validation,
List<String> validOptions) {
@ -210,6 +219,7 @@ public class ArgumentDefinition {
this.isHidden = isHidden;
this.componentType = componentType;
this.exclusiveOf = exclusiveOf;
this.otherArgumentRequired = otherArgumentRequired;
this.validation = validation;
this.validOptions = validOptions;
}
@ -275,6 +285,16 @@ public class ArgumentDefinition {
return exclusiveOf.trim().length() > 0 ? exclusiveOf.trim() : null;
}
/**
* Specifies other argument that is required to be used in conjunction with this argument.
* @param annotation Original field annotation.
* @return A required argument, or null if none are present.
*/
public static String getOtherArgumentRequired( Annotation annotation ) {
String otherArgumentRequired = ((String) CommandLineUtils.getValue(annotation, "otherArgumentRequired")).trim();
return otherArgumentRequired.length() > 0 ? otherArgumentRequired : null;
}
/**
* A regular expression which can be used for validation.
* @param annotation Original field annotation.

View File

@ -166,6 +166,7 @@ public abstract class ArgumentTypeDescriptor {
source.isHidden(),
makeRawTypeIfNecessary(getCollectionComponentType(source.field)),
ArgumentDefinition.getExclusiveOf(argumentAnnotation),
ArgumentDefinition.getOtherArgumentRequired(argumentAnnotation),
ArgumentDefinition.getValidationRegex(argumentAnnotation),
getValidOptions(source) );
}

View File

@ -75,6 +75,13 @@ public @interface Input {
*/
String exclusiveOf() default "";
/**
* Does this command-line argument require another argument.
* This will be the other argument required (presently only supports one).
* @return A string with the other required argument.
*/
String otherArgumentRequired() default "";
/**
* Provide a regexp-based validation string.
* @return Non-empty regexp for validation, blank otherwise.

View File

@ -82,6 +82,13 @@ public @interface Output {
*/
String exclusiveOf() default "";
/**
* Does this command-line argument require another argument.
* This will be the other argument required (presently only supports one).
* @return A string with the other required argument.
*/
String otherArgumentRequired() default "";
/**
* Provide a regexp-based validation string.
* @return Non-empty regexp for validation, blank otherwise.

View File

@ -270,7 +270,8 @@ public class ParsingEngine {
InvalidArgumentValue,
ValueMissingArgument,
TooManyValuesForArgument,
MutuallyExclusive }
MutuallyExclusive,
OtherArgumentRequired}
/**
* Validates the list of command-line argument matches.
@ -372,6 +373,26 @@ public class ParsingEngine {
if( !invalidPairs.isEmpty() )
throw new ArgumentsAreMutuallyExclusiveException( invalidPairs );
}
// Find sets of options that have other required arguments
if( !skipValidationOf.contains(ValidationType.OtherArgumentRequired)) {
Collection<ArgumentMatch> missingRequiredPairs = new ArrayList<ArgumentMatch>();
for( ArgumentMatch argumentMatch: argumentMatches.findSuccessfulMatches()) {
if(argumentMatch.definition.otherArgumentRequired != null) {
boolean otherRequiredArgumentSeen = false;
for(ArgumentMatch otherRequiredMatch: argumentMatches.findSuccessfulMatches()) {
if(argumentMatch.definition.otherArgumentRequired.equals(otherRequiredMatch.label)) {
otherRequiredArgumentSeen = true;
}
}
if(!otherRequiredArgumentSeen) {
missingRequiredPairs.add(argumentMatch);
throw new OtherRequiredArgumentMissingException(missingRequiredPairs);
}
}
}
}
}
/**
@ -814,6 +835,19 @@ class ArgumentsAreMutuallyExclusiveException extends ArgumentException {
}
class OtherRequiredArgumentMissingException extends ArgumentException {
public OtherRequiredArgumentMissingException( Collection<ArgumentMatch> arguments ) {
super( formatArguments(arguments) );
}
private static String formatArguments( Collection<ArgumentMatch> arguments ) {
StringBuilder sb = new StringBuilder();
for( ArgumentMatch argument: arguments )
sb.append( String.format("%nArguments '%s' and '%s' are required to go together.", argument.definition.fullName, argument.definition.otherArgumentRequired ) );
return sb.toString();
}
}
/**
* An exception for when an argument doesn't match an of the enumerated options for that var type

View File

@ -86,7 +86,7 @@ public class ParsingEngineUnitTest extends BaseTest {
MultiCharShortNameArgProvider argProvider = new MultiCharShortNameArgProvider();
parsingEngine.loadArgumentsIntoObject( argProvider );
Assert.assertEquals(argProvider.outputFile,"out.txt","Argument is not correctly initialized");
Assert.assertEquals(argProvider.outputFile, "out.txt", "Argument is not correctly initialized");
}
@ -114,11 +114,11 @@ public class ParsingEngineUnitTest extends BaseTest {
final String[] commandLine = new String[] {" --input_file ", "na12878.bam"};
parsingEngine.addArgumentSource( InputFileArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.parse(commandLine);
parsingEngine.validate();
InputFileArgProvider argProvider = new InputFileArgProvider();
parsingEngine.loadArgumentsIntoObject( argProvider );
parsingEngine.loadArgumentsIntoObject(argProvider);
Assert.assertEquals(argProvider.inputFile,"na12878.bam","Argument is not correctly initialized");
}
@ -167,7 +167,7 @@ public class ParsingEngineUnitTest extends BaseTest {
AllLociArgProvider argProvider = new AllLociArgProvider();
parsingEngine.loadArgumentsIntoObject( argProvider );
Assert.assertTrue(argProvider.allLoci,"Argument is not correctly initialized");
Assert.assertTrue(argProvider.allLoci, "Argument is not correctly initialized");
}
private class AllLociArgProvider {
@ -214,12 +214,12 @@ public class ParsingEngineUnitTest extends BaseTest {
public void enumMixedCaseTest() {
final String[] commandLine = new String[] { "--test_enum", "oNe" };
parsingEngine.addArgumentSource( EnumArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.addArgumentSource(EnumArgProvider.class);
parsingEngine.parse(commandLine);
parsingEngine.validate();
EnumArgProvider argProvider = new EnumArgProvider();
parsingEngine.loadArgumentsIntoObject( argProvider );
parsingEngine.loadArgumentsIntoObject(argProvider);
Assert.assertEquals(argProvider.testEnum, TestEnum.ONE, "Enum value is not correct");
}
@ -228,12 +228,12 @@ public class ParsingEngineUnitTest extends BaseTest {
public void enumDefaultTest() {
final String[] commandLine = new String[] {};
parsingEngine.addArgumentSource( EnumArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.addArgumentSource(EnumArgProvider.class);
parsingEngine.parse(commandLine);
parsingEngine.validate();
EnumArgProvider argProvider = new EnumArgProvider();
parsingEngine.loadArgumentsIntoObject( argProvider );
parsingEngine.loadArgumentsIntoObject(argProvider);
Assert.assertEquals(argProvider.testEnum, TestEnum.THREE, "Enum value is not correct");
}
@ -300,7 +300,7 @@ public class ParsingEngineUnitTest extends BaseTest {
final String[] commandLine = new String[0];
parsingEngine.addArgumentSource( RequiredArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.parse(commandLine);
parsingEngine.validate();
}
@ -345,10 +345,10 @@ public class ParsingEngineUnitTest extends BaseTest {
parsingEngine.addArgumentSource( RequiredArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.validate( EnumSet.of(ParsingEngine.ValidationType.MissingRequiredArgument) );
parsingEngine.validate(EnumSet.of(ParsingEngine.ValidationType.MissingRequiredArgument));
RequiredArgProvider argProvider = new RequiredArgProvider();
parsingEngine.loadArgumentsIntoObject(argProvider );
parsingEngine.loadArgumentsIntoObject(argProvider);
Assert.assertNull(argProvider.value, "Value should have remain unset");
}
@ -396,7 +396,7 @@ public class ParsingEngineUnitTest extends BaseTest {
@Test(expectedExceptions= ReviewedGATKException.class)
public void duplicateShortNameTest() {
parsingEngine.addArgumentSource( DuplicateShortNameProvider.class );
parsingEngine.addArgumentSource(DuplicateShortNameProvider.class);
}
@ -444,7 +444,7 @@ public class ParsingEngineUnitTest extends BaseTest {
final String[] commandLine = new String[] {"--value","1","--value","2","--value","3"};
parsingEngine.addArgumentSource( RequiredArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.parse(commandLine);
parsingEngine.validate();
}
@ -491,7 +491,7 @@ public class ParsingEngineUnitTest extends BaseTest {
@Test
public void correctDefaultArgNameTest() {
parsingEngine.addArgumentSource( CamelCaseArgProvider.class );
parsingEngine.addArgumentSource(CamelCaseArgProvider.class);
DefinitionMatcher matcher = ArgumentDefinitions.FullNameDefinitionMatcher;
ArgumentDefinition definition = parsingEngine.argumentDefinitions.findArgumentDefinition("myarg", matcher);
@ -510,7 +510,7 @@ public class ParsingEngineUnitTest extends BaseTest {
final String[] commandLine = new String[] {"--mybool", "true"};
parsingEngine.addArgumentSource( BooleanArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.parse(commandLine);
parsingEngine.validate();
}
@ -524,14 +524,14 @@ public class ParsingEngineUnitTest extends BaseTest {
public void validParseForAnalysisTypeTest() {
final String[] commandLine = new String[] {"--analysis_type", "Pileup" };
parsingEngine.addArgumentSource( AnalysisTypeArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.validate( EnumSet.of(ParsingEngine.ValidationType.MissingRequiredArgument) );
parsingEngine.addArgumentSource(AnalysisTypeArgProvider.class);
parsingEngine.parse(commandLine);
parsingEngine.validate(EnumSet.of(ParsingEngine.ValidationType.MissingRequiredArgument));
AnalysisTypeArgProvider argProvider = new AnalysisTypeArgProvider();
parsingEngine.loadArgumentsIntoObject( argProvider );
parsingEngine.loadArgumentsIntoObject(argProvider);
Assert.assertEquals(argProvider.Analysis_Name,"Pileup","Argument is not correctly initialized");
Assert.assertEquals(argProvider.Analysis_Name, "Pileup", "Argument is not correctly initialized");
}
private class AnalysisTypeArgProvider {
@ -578,6 +578,36 @@ public class ParsingEngineUnitTest extends BaseTest {
Integer bar;
}
@Test(expectedExceptions=OtherRequiredArgumentMissingException.class)
public void otherArgumentRequiredTestWithoutRequiredArguments() {
String[] commandLine = new String[] {"--foo","5"};
parsingEngine.addArgumentSource( OtherArgumentRequiredArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.validate();
}
@Test
public void otherArgumentRequiredTestWithRequiredArguments() {
String[] commandLine = new String[] {"--foo","5", "--bar", "6"};
parsingEngine.addArgumentSource( OtherArgumentRequiredArgProvider.class );
parsingEngine.parse( commandLine );
parsingEngine.validate();
}
@SuppressWarnings("unused")
private class OtherArgumentRequiredArgProvider {
@Argument(doc="foo", otherArgumentRequired="bar")
Integer foo;
@Argument(doc="bar",required=false)
Integer bar;
@Argument(doc="OtherIrrelevantArgument", required = false)
Integer OtherIrrelevantArgument;
}
@Test(expectedExceptions=InvalidArgumentValueException.class)
public void argumentValidationTest() {
// Passing only foo should work fine...