Open jairideout opened 10 years ago
:+1:
Would we remove the ability of Interfaces to define the available choices? I think there might be some use case in allowing choices for situations where Parameter=None
. Falling back to my old example, choosing between xml, and json formatted output, which is definitely in the domain of an OutputHandler
and it's InputName
. Additionally bool
is handled using a modified Choice list in the HTMLInterface which is convenient, but not strictly required.
But I really like the idea of Commands
being 'choice' aware!
I like your idea of letting interfaces define available choices if there isn't an associated Parameter. They could also override a Parameter's choices, like we're currently doing with Required, Help, and Default. The default behavior would use the Parameter's available choices if an option has a Parameter, which would help us avoid the enum replication that's happening now. Sound good?
I like it! Would this also be a good time to expand the capacity of choice? Perhaps a boolean that would describe whether it was multiple-selections or single choice. Or should that be it's own type?
Actually now that I think about it, the semantic clarity of having a different type for multiple_selection outweighs the potential for code reuse. Also it would make the literal python type which the command accepts a little fuzzier than it needs to be.
:+1: for having two types (though single choice should be highest priority)
by highest priority I mean the first thing to be implemented since we have use-cases for this type already
So I am working through some of the changes we need to make to support "multiple_choice" and it looks like there is a ValidateValue
on Parameter
however the CommandIn
constructor doesn't ever provide a value, so it is always None
. The ValidateValue
is "used" in _validate_kwargs
and performs the functionality we need. So should I let CommandIn
provide a proper ValidateValue
function to the Parameter
or should I just put the value check inside _validate_kwargs
since ValidateValue
doesn't seem to be used for anything presently?
Yeah, I don't think ValidateValue
is being used anywhere yet. I don't have very strong feelings on this, but I'd prefer that we leave ValidateValue
alone so that it can be used for other (custom) validation in the future. This also has the advantage of being more explicit by separating validation of choices from other types of validation (which we're currently doing for validation of Required
).
Were you thinking of adding a new attribute to CommandIn
that defines the list of acceptable values (e.g., CommandIn.Choices
) or were you planning on subclassing CommandIn
? I can see arguments for either approach.
Assigning this issue to you
I don't think subclassing makes much sense, a particular DataType
shouldn't need its own commandin class. I will let ValidateValue
pass through to the Parameter
so it can be used, but I will find a different way to validate for native types.
Good point, that makes sense. Thanks!
@ebolyen There is any update on this? I'm working on adding the choice
and multiple_choice
support to optparse
and I'm wondering if this issue will change my implementation (see #251).
It'd be great to add support for input that can be one of several controlled values, like optparse's
choice
type. For some concrete usage examples, take a look at biom-format'sbiom convert
command and corresponding Commandbiom.commands.table_converter
(particularly the optionstable_type
andprocess_obs_metadata
). Right now the Command'srun
method performs manual checks to ensure the input is in the acceptable list of choices.HTMLInterface has support for this type, but it'd be really cool to have this supported at the Command level instead of at the interface level. That way, the Command could automatically check that an input is valid, as well as auto-generate the help text for the input (e.g., by appending a description of the acceptable values to the input's help text).
Another advantage for pushing this to the command level is that the functionality can be reused across interfaces (e.g., optparse, HTMLInterface) and we don't have the controlled vocabulary replicated in multiple places.