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

Make conditional input parameters editable at runtime? #4528

Open
TKlingstrom opened this issue Aug 31, 2017 · 16 comments
Open

Make conditional input parameters editable at runtime? #4528

TKlingstrom opened this issue Aug 31, 2017 · 16 comments

Comments

@TKlingstrom
Copy link

I think we should consider if conditional input parameters should be configurable at runtime. Its not really a right or wrong question but about prioritising how actively workflow users should be guided. In the docs conditional parameters are recommended for things such as creating a tool for selecting reference data (https://docs.galaxyproject.org/en/master/dev/schema.html#tool-inputs-conditional).

A common use of the conditional wrapper is to select between reference data managed by the Galaxy admins (for instance via data managers ) and history files. A good example tool that demonstrates this is the Bowtie 2 wrapper.

In practice this mean that every tool which relies on reference data and is made according to the instructions will lock all workflows using it to a specific genome build. For researchers working in an environment with several animal models or microbiology this create a major usability issue.

When deciding on the issue there are two ways to prioritise when deciding if they should be editable or not.

  1. Focusing on minimising the risk of users breaking workflows by selecting inappropriate options at runtime.
  2. Ensure that users finds it meaningful to create Galaxy workflows for later usage by themselves or others.

If we prioritise option workflow being flexible (ie conditional parameters being editable) there are two issues:
A) Users may at runtime select to remove an output that is used later in the workflow. Yielding a message stating along the lines of "missing dataset in step X". The history will be color coded to show which step fails and error testing is identical to if a tool have failed to produce output.
B) Users may be confused if they are allowed to change input in a tool like BWA from provided reference genomes.

If we instead prioritise to lower the risk of user caused errors then we have one issue:

  • It becomes less attractive for people to develop and share pipelines as their usage scope become unnecessarily narrow unless the final users are trained to edit workflows. .

Personally I work at an agricultural university with many small groups doing similar things but on different animals/microbes so for me flexibility is very important. Given the user friendly way workflows seem to fail when lacking data I do however think that the risk of issues caused by workflow failures are rather small.

@mvdbeek
Copy link
Member

mvdbeek commented Aug 31, 2017

OK, that's not entirely correct, you can set non-conditional select boxes at runtime, see for example the genome selection in bowtie2. It's just the conditional itself that can't be set at runtime (For a good reason -- if you switch to a user-provided reference genome, you'll need another input file).

@TKlingstrom
Copy link
Author

I've tested things now and apparently bamCoverage work differently than bowtie2. In bowtie2 I can change the reference genome in bamCoverage I cannot. So I guess the issue conditional is implemented inappropriately for that particular tool.

So the issue is less severe than it first appeared but I still think the overall judgement is a bit odd. If you alter a workflow to use a user-provided reference genome it is self-evident that you cannot run the workflow if you cannot provide one. On the other hand if the workflow is developed for a user-provided genome and I wish to switch to a default one, then it would be good if I can do so.

@nsoranzo
Copy link
Member

nsoranzo commented Sep 1, 2017

@TKlingstrom If you are referring to the Effective genome size parameter of bamCoverage, it is implemented as a conditional param (so not editable at workflow run, as explained by @mvdbeek) because if you select specific it will let you specify the effective genome size in an additional text box, see https://github.com/fidelram/deepTools/blob/master/galaxy/wrapper/deepTools_macros.xml#L550 .

Maybe this limitation can be restricted to conditionals that may contain a param of type data, data_collection or similar? Ping @guerler

@guerler
Copy link
Contributor

guerler commented Sep 1, 2017

@nsoranzo same thought here. I agree.

@TKlingstrom
Copy link
Author

@nsoranzo, I got a slightly tangential question regarding how it works right now.

When coding the wrapper you can decide on either making it a decision tree with several levels or provide all choices at a single level. Bowtie2 use alternative 1 described below and bamCoverage uses alternative 2 described below.

Alternative 1, conditional parameter followed by a second level dependent on level 1.

Level 1
Let user decide based on if they want to load predefined data or submit their own.

Level 2
If option pre-defined: Select list
If option user input: Upload form/text field

--> This mean that the user get either a list that can be selected from or the option to make user based input in whatever format the tool supports.

Alternative 2: You write the options in a single level where the worfklows become pre-defined with a select option or accepts a user input in the form of a text from a text field (or a file I guess if you have that option for the tool).

Level 1:
Select option 1
Select option 2
Etc
User input option.

--> This mean that the user cannot switch between predefined inputs at run time, but user defined input can still be an option.

As a novice to Galaxy I lack the overview of the impact of many design decisions. But from my position it looks like alternative 1 is preferable when configuring tools?

Regarding the issue here I think that instead of restricting conditional parameters if they accept data or dataset collections there could simply be a small warning stating that the workflow will fail if users change the parameter and does not provide the appropriate data themselves.

@nsoranzo
Copy link
Member

nsoranzo commented Sep 5, 2017

@TKlingstrom Your analysis seems correct to me and I can say that the vast majority of tools use your "alternative 1" for genome selection.

Regarding the issue here I think that instead of restricting conditional parameters if they accept data or dataset collections there could simply be a small warning stating that the workflow will fail if users change the parameter and does not provide the appropriate data themselves.

I think that's a bit more complicated than that, unfortunately.

@hexylena
Copy link
Member

xref #979 which I'm closing since it's a dupe (well, it was the original issue) but has less useful discussion.

@hexylena
Copy link
Member

xref #1132 which was mentioned on that issue as well.

@FredericBGA
Copy link
Contributor

Is there a workaround for this? I really need to let users choose a locally Blast database, a Blast database from their history etc.

@mvdbeek
Copy link
Member

mvdbeek commented Jun 26, 2019

You can select parameters (like indexed blast databases) in the workflow run page, what you can't do is switch between pre-indexed databases and creating databases from an input file. For that you'll have to create a separate workflow. If you want to share the common parts you can put the common parts in one workflow and then include that workflow in another workflow.

@FredericBGA
Copy link
Contributor

ok, I've just created a tool in order to let the user choose between:

  • paste a sequence in a text area
  • paste IDs in a text area, then IDs are searched and it creates a file with sequences
  • select a file with sequences
  • select a file with IDs, then IDs are searched ans it creates a file with sequences

it works well as a tool, with a conditional. But I can't use it in a workflow...

@FredericBGA
Copy link
Contributor

#8218 could help to solve this. I can put several inputs, optional, make a concatenate and work with the result!

@FredericBGA
Copy link
Contributor

What I have done so far:

  • make all inputs of my tool optional="True"
  • Add this tool 4 times in the workflow
    • each occurrence represents one choice of the conditionnal
  • Run a concatenate dataset woth the 4 outputs, only one will be filled

It works.

@bernt-matthias
Copy link
Contributor

In practice this mean that every tool which relies on reference data and is made according to the instructions will lock all workflows using it to a specific genome build.

might be solved by this #8599

...?

@bernt-matthias
Copy link
Contributor

Forget about my previous comment .. I guess the PR is unrelated.

Anyway I think it would be a great to allow to change conditionals in the workflow run form:

Just two tools that use conditionals to present only relevant parameter choices:

https://github.com/galaxyproject/tools-iuc/blob/d6e367b8f4cdf7b74654e7bae9dd206a85a52665/tools/stacks2/stacks_procrad.xml#L50
https://github.com/galaxyproject/tools-iuc/blob/d6e367b8f4cdf7b74654e7bae9dd206a85a52665/tools/dada2/dada2_filterAndTrim.xml#L91

But I understand the problems with mandatory (data/workflow) inputs in the conditionals and outputs that might be filtered by options in the conditionals.

@mvdbeek
Copy link
Member

mvdbeek commented Sep 30, 2021

Maybe this limitation can be restricted to conditionals that may contain a param of type data, data_collection or similar? Ping @guerler

Since it came up again today, yes that is probably a good idea, however we may also have output filters that work on values within the conditional when, so that's always going to be risky and not straightforward to check.

Although outputs can also be filtered based on parameters outside of conditionals ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants