Even Richer Workflow Inputs #9086

jmchilton · 2019-12-10T04:23:58Z

Builds on #9073 (Richer Workflow Inputs).

Adds:

Ability to specify formats for data and collection inputs. Currently collections have no way to restrict formats - and data parameters will be restricted automatically but workflow authors may be able to provide more precise, correct data types than Galaxy can (or does) infer. Formats may be specified as a single value or a list of values in the corresponding workflow format.
Ability to add "restrictions" on text data inputs - so they appear as static select boxes from supplied key/values, values determined dynamically (this commit includes Implement 'select' Input Parameter in Workflow Editor #8264), or a text box with suggestions. These changes include changing the workflow editor logic so that text input can connect to select inputs.

I had wanted to use Repeats for these, but they don't work with the workflow module framework right now and it wasn't obvious how to adjust things so they would. Instead users supply these as common separated values in a text box. Since we're not persisting ideas related to the client plumbing into the database - there should be nothing to prevent us from re-working the interface to use different client form stuff in the future as long as we're on board with supporting the follow Format 2 syntax elements:

inputs:
  fasta_input:
     type: data
     format: fasta

inputs:
  sequences:
     type: data
     format:
      - fasta
      - fastqsanger

inputs:
  sequencer:
     type: text
     restrictions:
      - iseq
      - miniseq
      - miseq
      - nextseq
      - nanopore

inputs:
  sequencer:
     type: text
     restrictions:
      - value: iseq
        label: iSeq
      - value: miniseq
        label: MiniSeq
      - value: miseq
        label: MiSeq
      - value: nextseq
        label: NextSeq
      - value: nanopore
        label: Nanopore

inputs:
  sequencer:
     type: text
     restrictOnConnections: true

inputs:
  sequencer:
     type: text
     suggestions:
      - iseq
      - miniseq

jmchilton · 2019-12-10T14:33:18Z

Sleeping on this I think my new plan is to wrap the restrictions parameter in a conditional with four options:

Do not specify restrictions (default).
Attempt restriction based on connections.
Provide list of restricted values.
Provide list of suggested values.

For restrict based on select connections - I've rebased #8264 into a single commit with 3 authors 98c9b50 that I can apply to this and rework.

Plan for select merging would be I think:

Rework parameter finding to visit_input_values right (would eliminate a lot of the code).
Remove the select option - just make it part of the text option.

The corresponding Format 2 syntaxes for four options would be something like:

inputs:
  sequencer:
     type: text

inputs:
  sequencer:
     type: text
     restrictions:
      - iseq
      - miniseq

inputs:
  sequencer:
     type: text
     restrictOnConnections: true

inputs:
  sequencer:
     type: text
     suggestions:
      - iseq
      - miniseq

jmchilton · 2019-12-10T16:14:01Z

Well I messed up the cherry-pick because I started refactoring while I should have been merging the conflicts and committing first - so it is hard to see the lineage - but the authors of #8264 are on the commit and a good amount of the code is still there. Using visit_input_values really did reduce the amount of code needed for that.

I think otherwise the idea worked out:

I think a lot could go wrong when generating those values so the option in the GUI is called "Attempt restriction based on connections" and this code fallbacks to a plain text input if option generation or intersection fails.

A couple more ideas:

If just one connection is made, just use the options - because intersection breaks the ordering.
Allow restriction and suggestion lists to define key value pairs. This could be rich in the Format2 source and something like : key1:A description of key2, key2:A description of key2 in the client form.

mvdbeek · 2019-12-10T16:16:34Z

This looks amazing, and yeah, visit_input_values is the right thing.

- Allow parameter inputs to be optional. - Allow parameter inputs to have default values (if optional). - Allow data and collection parameter to be optional. - Unimplemented but new abstractions should allow having users specify extensions manually if they'd like for data and collection parameters. The previous workflow Module interface would more or less persist Galaxy's "Tool State" directly into the database. The tool state framework persists state in an un-typed fashion (e.g. the optional value for data inputs would be 'True' instead of True). The Format 2 YAML that we would like to publish to Galaxy and export from Galaxy would much more cleanly represent this as actual boolean values. The problem gets even worse with parameter inputs where we want the default value to be settable only if optional is True. This requires a conditional in the "Tool Form" language - so the persisted value would be nested structures without typing. For instance - instead of ``{default: 5, parameter_type: "integer", "optional": true}`` that we might want to store into the database or read from a workflow definition file, the framework would require something like ``{"parameter_type": "integer", "parameter_defintion": {"optional": {"optional": "True", {"specify_optional": {"specify_default": "True", "default": "5"}}}}}``. My preference may have been to have plugins defined in the client that just produce the minimal JSON - but I didn't know how to implement that and Sam's preference was that the client use the tool form framework for everything on the side panel anyway though. As a result, this commit adds abstractions to separate module tool state (for commiunicating with client and tied to tool form rendering) from abstract export state - loaded from the API when not coming from the tool and stored in database. To summarize workflow module changes... - module.get_inputs can build complex tool form parameteras (unchanged) - module.state is always the tool form state (unchanged) - module.get_export_state is added to get the abstract state for export and for reasoning about internally - module.recover_state now assumes recovery from abstract step state until from_tool_form=True is supplied - API entry points tied to the tool form (build_module) just pass through from_tool_form by default. - The client now sends from_tool_form into the API whenever it serializes the workflow state for re-saving

jmchilton · 2019-12-10T19:29:15Z

Having these constructs for building and configuring inputs and having the abstractions to separate that representation from whats gets persisted... combined with having expression tooling framework merged... we could finally build my arbitrary expression module with user described inputs and outputs...

That combined this with the conditionals proposal for CWL (common-workflow-language/cwl-v1.2#4) which is so clean, well thought through, and I think would map relatively cleanly on top of all of these new Galaxy constructs...

It isn't on the roadmap... but it is hard not to keep going 😆...

Rebased of dozens of commits together by jraysajulga, mvdbeek, and jjohnson. Co-Authored-By: Jim Johnson <[email protected]> Co-Authored-By: Marius van den Beek <[email protected]> Co-Authored-By: John Chilton <[email protected]>

mvdbeek · 2019-12-11T10:30:39Z

lib/galaxy/workflow/modules.py

+        collection_type = parameter_def["collection_type"]
+        optional = parameter_def["optional"]
+        collection_type_source = dict(name="collection_type", label="Collection type", type="text", value=collection_type)
+        collection_type_source["options"] = [


mvdbeek

This is awesome! There are still some rough edges (as expected), but we can fix these later IMO.
One thing we should do is disconnect optional parameter output nodes from inputs that are required, and we should have some more validation prior to launching the workflow.

lib/galaxy/tool_util/parser/factory.py

lib/galaxy/workflow/modules.py

jmchilton · 2019-12-11T13:16:30Z

One thing we should do is disconnect optional parameter output nodes from inputs that are required

Yeah... I forgot about the editor - the declared formats are also not used. I'll give a shot at these two and see what I come up with.

we should have some more validation prior to launching the workflow

Do you have a specific thing that wasn't caught?

mvdbeek · 2019-12-11T13:17:30Z

Yeah, making a dataset optional, then not providing it. Same with an optional integer parameter connected to a required value and not provided.

mvdbeek · 2019-12-11T13:18:11Z

But I'm happy to merge without this if this looks to be a bigger effort (I could also look into that).

jmchilton · 2019-12-11T16:12:15Z

First cut at the workflow editor. Handles optionals in data and collections, incorporates the format information. Handles updates to some degree. There are some corner cases I think - I'm not sure if they need to be addressed pre-merge:

Optional parameter types aren't validated at the connection level.
Switching format types or going from non-optional to optional on inputs could cause existing connections to become invalid. These aren't destroyed automatically.

The existing hack of extensions=['input'] is bad enough and should work fine for collections. Not sure what past-John was thinking by adding this.

mvdbeek · 2019-12-17T20:21:18Z

client/galaxy/scripts/mvc/workflow/workflow-terminals.js

@@ -370,6 +377,12 @@ var BaseInputTerminal = Terminal.extend({
            `Effective output data type(s) [${cat_outputs}] do not appear to match input type(s) [${this.datatypes}].`
        );
    },
+    _producesAcceptableDatatypeAndOptionalness(other) {
+        if (!this.optional && !this.multiple && other.optional) {


I guess that will work if a tool author properly handles the iteration, but in my tests I used the cat tool in https://toolshed.g2.bx.psu.edu/repository?repository_id=2593fd36ae8011aa which does

#echo ' '.join(['"%s"' % $file for $file in $inputs])#

which evaluates to "None". I guess the proper way is #echo ' '.join(['"%s"' % $file for $file in $inputs if $file])# or make sure that an empty multi-data parameter behaves like [].
The tool form doesn't let you do this and insists on passing a dataset ... thought I can see this being useful.
So I guess this is fine for now and we should see if it is possible to fix this style of iteration on the Galaxy side ?

The tool form insist on a dataset even if it is marked as optional? I thought I hacked around this years ago for Dan... ugh

it's the other way round, this code allows connecting optional datasets to required multi-data parameters. I think there is some merit in not allowing that, as the tool form does.

Oh - that is 100% intentional on my part. You should be able to connect two optional data parameters to a required multi-parameter - in case one of them is set, right? The tool form lets you do this - run two tools and pick the output that is produced manually and send it to the next tool 😃. This should be caught at runtime IMO so you can do conditionally things

mvdbeek · 2019-12-17T20:26:23Z

Awesome, this is a big step!

guerler · 2020-01-02T18:02:48Z

lib/galaxy/workflow/modules.py

+                when_restrict_static_suggestions.value = "staticSuggestions"
+                when_restrict_static_suggestions.inputs = OrderedDict()
+
+                # Repeats don't work - so use common separated list for now.


Why do repeats not work?

They don't - I don't know why exactly. I spent a few hours trying to walk through what was happening with the parameter expansion logic and trying a variety of tweaks and was pretty convinced it wasn't a trivial amount of code away from working.

We could do some cool stuff around allowing users to declare expressions with multiple inputs and outputs if we could get that to work.

jmchilton added kind/enhancement area/workflows labels Dec 10, 2019

galaxybot added the status/WIP label Dec 10, 2019

jmchilton mentioned this pull request Dec 10, 2019

Implement 'select' Input Parameter in Workflow Editor #8264

Closed

11 tasks

jmchilton added 4 commits December 10, 2019 13:24

Skip recording output values if unlabeled (like data/collections).

02440b6

Restricted selection text inputs for workflow inputs.

3654901

Allow specifying format on data/collection workflow inputs.

2819619

jmchilton force-pushed the revise_workflow_inputs_2 branch from aadefeb to 10807cd Compare December 10, 2019 19:15

jraysajulga and others added 4 commits December 10, 2019 14:31

Add select list option to workflow editor.

36d7b07

Rebased of dozens of commits together by jraysajulga, mvdbeek, and jjohnson. Co-Authored-By: Jim Johnson <[email protected]> Co-Authored-By: Marius van den Beek <[email protected]> Co-Authored-By: John Chilton <[email protected]>

Optimize user experience of connecting to one option.

b9cf792

Allow labels on restriction/suggestion values.

de1e11c

Fix language a bit in restriction type selection box.

36bc4e3

jmchilton force-pushed the revise_workflow_inputs_2 branch from 10807cd to 36bc4e3 Compare December 10, 2019 19:32

Fix unit tests.

20d737c

jmchilton changed the title ~~[WIP] Even Richer Workflow Inputs~~ Even Richer Workflow Inputs Dec 11, 2019

jmchilton removed the status/WIP label Dec 11, 2019

galaxybot added this to the 20.01 milestone Dec 11, 2019

mvdbeek reviewed Dec 11, 2019

View reviewed changes

mvdbeek approved these changes Dec 11, 2019

View reviewed changes

lib/galaxy/tool_util/parser/factory.py Show resolved Hide resolved

lib/galaxy/workflow/modules.py Outdated Show resolved Hide resolved

Update docstring for get_input_source.

1fd3b9f

jmchilton mentioned this pull request Dec 11, 2019

Eliminate extensions=['input_collection'] hack. #9095

Closed

jmchilton added 5 commits December 11, 2019 12:26

Dictified representation instead of XML for more module param decls.

4ec4f3c

Eliminate extensions=['input_collection'].

26d5150

The existing hack of extensions=['input'] is bad enough and should work fine for collections. Not sure what past-John was thinking by adding this.

Use annotated format information in workflow editor.

7ba061e

Update workflow editor for optional outputs.

44b63ff

Eliminate some uses of self in workflow editor tests.

68e6d8f

jmchilton force-pushed the revise_workflow_inputs_2 branch from 4e38e8b to ec450f2 Compare December 11, 2019 17:26

Stylistic improvements to workflow editor JS.

383034b

jmchilton force-pushed the revise_workflow_inputs_2 branch from ec450f2 to 383034b Compare December 11, 2019 18:30

This was referenced Dec 14, 2019

Simplified Workflow Run Component #9111

Closed

Implement Collection Selection Dialog #9114

Open

mvdbeek reviewed Dec 17, 2019

View reviewed changes

mvdbeek merged commit 9c46e85 into galaxyproject:dev Dec 17, 2019

guerler reviewed Jan 2, 2020

View reviewed changes

nsoranzo deleted the revise_workflow_inputs_2 branch June 17, 2020 11:57

This was referenced Sep 18, 2020

Default values for workflow parameters #7775

Closed

Adding "select" as parameter type of input parameters #8159

Closed

jmchilton mentioned this pull request Oct 21, 2020

[Workflows] Optional collection input #8576

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Even Richer Workflow Inputs #9086

Even Richer Workflow Inputs #9086

jmchilton commented Dec 10, 2019 •

edited

Loading

jmchilton commented Dec 10, 2019

jmchilton commented Dec 10, 2019

mvdbeek commented Dec 10, 2019

jmchilton commented Dec 10, 2019

mvdbeek Dec 11, 2019

mvdbeek left a comment

jmchilton commented Dec 11, 2019

mvdbeek commented Dec 11, 2019

mvdbeek commented Dec 11, 2019

jmchilton commented Dec 11, 2019

mvdbeek Dec 17, 2019

jmchilton Dec 18, 2019

mvdbeek Dec 18, 2019

jmchilton Dec 19, 2019 •

edited

Loading

mvdbeek commented Dec 17, 2019

guerler Jan 2, 2020

jmchilton Jan 7, 2020

Even Richer Workflow Inputs #9086

Even Richer Workflow Inputs #9086

Conversation

jmchilton commented Dec 10, 2019 • edited Loading

jmchilton commented Dec 10, 2019

jmchilton commented Dec 10, 2019

mvdbeek commented Dec 10, 2019

jmchilton commented Dec 10, 2019

mvdbeek Dec 11, 2019

Choose a reason for hiding this comment

mvdbeek left a comment

Choose a reason for hiding this comment

jmchilton commented Dec 11, 2019

mvdbeek commented Dec 11, 2019

mvdbeek commented Dec 11, 2019

jmchilton commented Dec 11, 2019

mvdbeek Dec 17, 2019

Choose a reason for hiding this comment

jmchilton Dec 18, 2019

Choose a reason for hiding this comment

mvdbeek Dec 18, 2019

Choose a reason for hiding this comment

jmchilton Dec 19, 2019 • edited Loading

Choose a reason for hiding this comment

mvdbeek commented Dec 17, 2019

guerler Jan 2, 2020

Choose a reason for hiding this comment

jmchilton Jan 7, 2020

Choose a reason for hiding this comment

jmchilton commented Dec 10, 2019 •

edited

Loading

jmchilton Dec 19, 2019 •

edited

Loading