choice option type with the append action should be used for passing metrics to scripts about qiime HOT 26 CLOSED

Imported from trac issue 10. Created by caporaso on 2011-04-22T14:53:18, last modified: 2011-05-24T16:34:34

• caporaso attached proposed_new_cogent_option_types.py on 2011-04-22 14:53:27

from qiime.

I think this should involve adding this functionality to a few scripts, and then updating the PyCogent scripting guidelines to recommend usage of this feature.

from qiime.

@josenavas, here's an example of using the append action with the choice type:

make_option('-m', '--metric', type='choice', action='append', choices=['foo', 'bar', 'baz'],
    help='metric(s) to use [default: %default]', default=[])

I don't have this hooked up to any scripts yet, but does this give you something to test with Galaxy?

from qiime.

Yes, I think this will be enough to test with Galaxy. If it works, I'll be modifying the affected scripts.

from qiime.

Great- do you want to assign the issue to yourself? I haven't started any work on it yet.

from qiime.

Yep, It could be assigned to me.

[Done]

from qiime.

This solution perfectly works with the Galaxy integration code. I will be adding an issue for each script that needs to be modified, this way it is easier to keep track of those scripts.
What do you think it's the best solution: create a single pull request that resolves all the scripts or create a pull request for each issue/script? I think the last solution will be a cleaner solution, although it will increase the number of pull requests...

from qiime.

Sorry to intrude in this discussion but I would suggest creating a single new issue where you have a check-list containing all the scripts+options that need to be changed and then issue individual pull requests per item in the check-list, that way not only you can fix this but other developers can help and be aware of the overall progress of this refactioring.

from qiime.

I'm a bit confused with your proposal. As far I understand, I should create a single issue with a check-list with all the scripts+options that need to be fixed, and then create a single pull-request for each of these items, right? On the link you posted shows an example where the check-list is in the pull-request, but if we use this approach, I wll be the only one that can add more commits to these pull-request, AFAIK.

That's why I'm a bit confused and I want to clarify it before creating any issue/pull-request.

from qiime.

Hmm let me know if I'm missing something from your last e-mail, but what I meant was that:

The list should be in the dedicated issue. Then as shown in the small animation, when someone issues a pull request, that dev could reference an item from the list in the dedicated issue. This is similar to what people do referencing an issue from a pull request, but with a different level of granularity.

I mentioning this option as it seems like a good fit for this problem, however there are different ways to keep track of the things to be done.

Yoshiki Vázquez Baeza

El 16/01/2013, a las 01:24 p.m., josenavas [email protected] escribió:

I'm a bit confused with your proposal. As far I understand, I should create a single issue with a check-list with all the scripts+options that need to be fixed, and then create a single pull-request for each of these items, right? On the link you posted shows an example where the check-list is in the pull-request, but if we use this approach, I wll be the only one that can add more commits to these pull-request, AFAIK.

That's why I'm a bit confused and I want to clarify it before creating any issue/pull-request.

—
Reply to this email directly or view it on GitHub.

from qiime.

That checklist feature looks awesome - this seems like a great opportunity to try it out.

from qiime.

I've playing around that and I found the following issue:

Using the append action the user cannot supply a comma-separated list in order to represent the multiple values, what the user has to do is tu supply multiple times the same option. For example, instead of calling:

alpha_diversity.py -i otu_table.biom -m chao1,PD_whole_tree -o adiv_chao1_pd.txt -t rep_set.tre

the user must specify:

alpha_diversity.py -i otu_table.biom -m chao1 -m PD_whole_tree -o adiv_chao1_pd.txt -t rep_set.tre

Since it changes the command line user interface, I preffer to ask if we should continue with that solution o we should move to a solution where we create a new option type as I suggested before, and we do not change the user behavior.

What do you think?

from qiime.

Following this discussion
https://groups.google.com/forum/?fromgroups=#!topic/qiime-developers/tV-zFfbbzwM
(note that you need to be a member of the group to see the discussion)
I
think the best will be to modify all documentation and scripts to use the
new format but still accept both. Any other opinions?

from qiime.

Yeah, if you make the action append for these, but still split each value on commas when you read option in, that should work to support both ways of passing multiple values.

from qiime.

The problem is that the code for the action append is in optparse and it does not accept comma separated items. Furthermore, the option checking is performed before we have any control on the main program. If we want to support both formats, we should create our own option type...

As another issue with the append action, Jai and I have found that if you specify a default value for that option, the user specified value will be appended to the default ones. A solution for that is not to specify any default value in the make_option call and add them in the main routine if the users didn't specify anything...

from qiime.

optparse and it does not accept comma separated items

Ah, I see. So this really would be a command-line interface change. If we want to go this route, it is absolutely essential that all scripts where this change is made are noted in ChangeLog, and that we alert users via the next release blog post. What do others think: should we make this change? Or should we create a new option type? @josenavas, have you tried creating the new option type to see if it is easy?

solution for that is not to specify any default value

That solution works. Note that you would still want to list what the defaults are in the help associated with that option. You would just do that as [default: weighted_unifrac,unweighted_unifrac] rather than [default: %default]. Annoying, but (kind of) makes sense.

from qiime.

Sorry, updated opinion on this and rather that just edit my previous comment I'll post a new one. I think the first thing to do is try creating a new option type for this. If that doesn't looking like it'll work then let's think about an interface change.

from qiime.

Ok, I will create the new option type ASAP. I will try to allow the comma separated list and the semicolon separated list (AFAIK, these are the two accepted formats).

I've tried to create the new option type in the past and it is pretty easy.

At this point, I'll create the new option type in pycogent backports (since the option types are in pycogent, and that should moved to pycogent later). Makes sense?

from qiime.

I don't think semi-colon separated lists are used anywhere, are they? bash separate those into separate commands.

At first just see if you can create the option and get it to work how you want, e.g. in a gist - if it's looks like it'll work we can get it into pycogent and pycogent backports.

from qiime.

Semicolons are used in some filter_XXX.py commands to separate category:values (VALID_STATES option).

El 07/03/2013, a las 18:40, Greg Caporaso [email protected] escribió:

I don't think semi-colon separated lists are used anywhere, are they? bash separate those into separate commands.

At first just see if you can create the option and get it to work how you want, e.g. in a gist - if it's looks like it'll work we can get it into pycogent and pycogent backports.

—
Reply to this email directly or view it on GitHub.

from qiime.

Oh, right, but only when the option is in quotes.

from qiime.

I've a branch in my repository with the new option type (multiple_choice) and I've modified alpha_diversity.py in order to make use of the new option type. The new option type supports comma- and semicolon-separated lists; and returns the values as a list, instead as a comma-separated list.

Should I make a pull request so everybody can check it or you can access directly to my branch (a2446ae)?

from qiime.

I think a pull request would be perfect - thanks!

from qiime.

I think this issue can be closed, since we've already created the new 'multiple_choice' option type. We should modify the PyCogent scripting guidelines, but I think it's easier if we create a new issue in PyCogent, which can be assigned to me.

from qiime.

OK, can you create that issue @josenavas?

from qiime.

Done

from qiime.