Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add argument to tool parameters. #8

Merged
merged 1 commit into from
Apr 9, 2015

Conversation

jmchilton
Copy link
Member

  • Expose via the API to help command-line construction.
  • Default the name of a parameter to be this (minus dashes) if name is not otherwise found.

@dannon
Copy link
Member

dannon commented Mar 4, 2015

This is an automated message. Thanks for your contribution, a Trello card to track this issue has been created. Apply this patch for testing.

@dannon
Copy link
Member

dannon commented Mar 5, 2015

Is there a missing commit here? On initial startup of Galaxy I'm seeing all tool parsing fail because no input source has an argument attribute.

 - Expose via the API to help command-line construction.
 - Default the name of a parameter to be this (minus dashes) if name is not otherwise found.
@jmchilton
Copy link
Member Author

None of the tools in Galaxy load you say? Huh - thanks - nice catch - such a subtle problem though I am not surprised I missed it.

I've updated the PR to be much less obviously broken.

argument = input_source.get("argument")
if argument:
name = argument.lstrip("-")
else:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmchilton this means that you can create a param without a name and it will take the stripped argument?
I don't think we should allow this. name should be required, argument and be optional and needs to be unstripped (for UI and gsh.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't modify argument - to me it makes sense that if you specify the argument as --input - the name can safely be inferred to be input. This provides some actual convenience to tool authors annotating things this way - argument will still be --input.

Put another way...

<param name="input" argument="--input"... 

feels redundant right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd put it the other way 'round. You can always refer to a parameter as
--{name} (as an argument) if argument isn't specified. Name is always
required.

On Fri, Mar 6, 2015 at 4:08 PM John Chilton notifications@github.com
wrote:

In lib/galaxy/tools/parameters/basic.py
#8 (comment):

@@ -235,6 +234,17 @@ def build( cls, tool, param ):
else:
return parameter_types[param_type](tool, param)

  • @classmethod
  • def parse_name(cls, input_source):
  •    name = input_source.get("name", None)
    
  •    if name is None:
    
  •        argument = input_source.get("argument")
    
  •        if argument:
    
  •            name = argument.lstrip("-")
    
  •        else:
    

It shouldn't modify argument - to me it makes sense that if you specify
the argument as --input - the name can safely be inferred to be input.
This provides some actual convenience to tool authors annotating things
this way - argument will still be --input.

Put another way...

feels redundant right?


Reply to this email directly or view it on GitHub
https://github.com/galaxyproject/galaxy/pull/8/files#r25978130.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see. Thought this will modify argument. Nice! 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All existing tools would have parameters that are then annotated as having arguments that may have nothing to do with the underlying application or wrapper then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, I see what you guys are trying to do now, I think. So, yeah, that wouldn't work. I still don't like making name optional (if you have the optional argument).

@martenson
Copy link
Member

This is good to go, right @jmchilton ?

@jmchilton
Copy link
Member Author

@martenson Lets give it a couple more days - I have some ideas to make it more clearly awesome and James mentioned he may have issues (though he was coming around on the idea last time I heard).

@bgruening
Copy link
Member

@guerler is it possible to parse a tool parameter for argument="" and add the (--parameter) string at the end of the help="" automatically?

@guerler
Copy link
Contributor

guerler commented Jun 12, 2015

@bgruening Parse it from where? The tool form? And what do you mean with end of help? Do you mean a separate section, listing all argument names? I need a few more details here. (:

@hexylena
Copy link
Member

@guerler from the argument parameter of the parameter definition.

E.g.:

  <param name="s" argument="-s" label="Some Parameter" help="wibbles wobbles" type="text" />

would automatically be rendered as:

  <param name="s" argument="-s" label="Some Parameter" help="wibbles wobbles (-s)" type="text" />

from the value of the argument parameter.

@bgruening
Copy link
Member

@guerler In our best practice guide we state that we want to have (--argument) somewhere mentioned. http://galaxy-iuc-standards.readthedocs.org/en/latest/best_practices/tool_xml.html#parameter-help
Would be nice if the toolform can handle this automatically, either by attaching it to the normal help text or by hovering over an info button(?). You are more creative than me ... but we want the original parameter name available in to toolform, if possible.

@guerler
Copy link
Contributor

guerler commented Jun 12, 2015

Thanks @erasche. Yes, this is possible. One option would be to make sure that the --argument is parsed through the tools parameters to_dict() function, and then we could attach it to the tool parameters help text on the front-end.

@jmchilton
Copy link
Member Author

Thanks @guerler - I believe it is in the to_dict function.

@guerler
Copy link
Contributor

guerler commented Jun 12, 2015

Yeah, I think so too. I'll look into it.

jmchilton added a commit that referenced this pull request Jun 14, 2015
hexylena added a commit that referenced this pull request Sep 14, 2015
Fix wrong local import of galaxy.
@jmchilton jmchilton deleted the argument_name branch November 25, 2015 15:05
dannon added a commit that referenced this pull request Jan 5, 2016
Minor tweaks to interactive tours backend.
jmchilton pushed a commit that referenced this pull request Feb 3, 2016
Warn on optional select test parameter.
mvdbeek pushed a commit that referenced this pull request Mar 4, 2016
Allow specifying tool_panel_section_label.
mvdbeek referenced this pull request in mvdbeek/galaxy Jan 24, 2017
Adapt examples to bioblend.objects .
jmchilton pushed a commit that referenced this pull request May 4, 2017
dannon pushed a commit that referenced this pull request Jun 1, 2018
Avoid using builtins for variable names.
dannon pushed a commit that referenced this pull request Sep 21, 2018
Consolidate portlet classes, fix tool form views
jmchilton pushed a commit that referenced this pull request Aug 22, 2019
migrating a few more interactive environments to interactive tools
chrisbarnettster added a commit to chrisbarnettster/galaxy that referenced this pull request Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants