Add workflow step parameters to workflow editor (try 3)#6925
Add workflow step parameters to workflow editor (try 3)#6925mvdbeek merged 20 commits intogalaxyproject:devfrom
Conversation
5d8be01 to
9e183de
Compare
9e183de to
05c405d
Compare
05c405d to
80c4e4d
Compare
| # and WorkflowRequestInputStepParameter objects. | ||
| for step in workflow.steps: | ||
| normalized_key = step.id | ||
| if step.type == "parameter_input": |
There was a problem hiding this comment.
This still trips up when there are data_input parameters in the workflow:
galaxy.workflow.run ERROR 2018-11-05 17:28:53,706 Failed to schedule Workflow[id=287,name=NonDataInputModule], problem occurred on WorkflowStep[index=1,type=data_input].
Traceback (most recent call last):
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 190, in invoke
incomplete_or_none = self._invoke_step(workflow_invocation_step)
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 266, in _invoke_step
use_cached_job=self.workflow_invocation.use_cached_job)
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/modules.py", line 535, in execute
progress.set_outputs_for_input(invocation_step, step_outputs)
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 393, in set_outputs_for_input
raise ValueError(message)
ValueError: Step with id 1008 not found in inputs_step_id ({1007: 'new label'})
galaxy.workflow.run ERROR 2018-11-05 17:28:53,708 Failed to execute scheduled workflow.
Traceback (most recent call last):
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 83, in __invoke
outputs = invoker.invoke()
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 190, in invoke
incomplete_or_none = self._invoke_step(workflow_invocation_step)
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 266, in _invoke_step
use_cached_job=self.workflow_invocation.use_cached_job)
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/modules.py", line 535, in execute
progress.set_outputs_for_input(invocation_step, step_outputs)
File "/Users/mvandenb/src/galaxy/lib/galaxy/workflow/run.py", line 393, in set_outputs_for_input
raise ValueError(message)
ValueError: Step with id 1008 not found in inputs_step_id ({1007: 'new label'})
There was a problem hiding this comment.
diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py
index ff49501943..1dd955dd9d 100644
--- a/lib/galaxy/workflow/run_request.py
+++ b/lib/galaxy/workflow/run_request.py
@@ -271,10 +271,13 @@ def build_workflow_run_configs(trans, workflow, payload):
# and WorkflowRequestInputStepParameter objects.
for step in workflow.steps:
normalized_key = step.id
- if step.type == "parameter_input":
+ if step.type in ["parameter_input", 'data_input']:
if normalized_key in param_map:
value = param_map.pop(normalized_key)
- normalized_inputs[normalized_key] = value["input"]
+ if step.type == 'data_input':
+ normalized_inputs[normalized_key] = value["input"]['values'][0]
+ else:
+ normalized_inputs[normalized_key] = value["input"]
steps_by_id = workflow.steps_by_id
# Set workflow inputs.
this let's me run the workflow -- but I'm guessing this will fall flat in many scenarios :/
There was a problem hiding this comment.
But the tests were passing and we weren't touching those inputs at all in this loop before, I don't get it. Would it only fail on workflows that had both data and parameter inputs?
There was a problem hiding this comment.
I'll write some tests and try to verify this is needed.
There was a problem hiding this comment.
Added a test case, I can verify you aren't crazy.
There was a problem hiding this comment.
Okay - I think the problem is here (https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/workflow/run.py#L393). inputs_by_id is now populated by only partially, but if outputs is supplied and has an 'output' value we can skip the if for that step. I can't make the change tonight, but I'll do it in the morning. I think this is a lighter touch than the PR - is that okay?
There was a problem hiding this comment.
Yeah, that's more than reasonable, I did play around this but didn't realize we could just skip if the output is there ... which makes a lot of sense.
|
@jmchilton with this jmchilton#70 I have successfully run a couple of workflows with input parameters, I'm not sure if that's the right way to go. |
In the current state when a text, integer, float, boolean or boolean parameter is set as "Set at Runtime" a new connectable input appears for the tool node, to which a Parameter can be properly dragged and connected. This still needs a little more separation from data inputs/outputs, and the new connection isn't persisted in a meaningful way.
select lists don't take a `value` argument.
Use ConnectedValue values to determine what non-data parameters to display connections for.
Don't treat data and non-data inputs/outputs different at the API boundary or at the workflow-manager.js level. Keep differences at the level of walking inputs/outputs in the workflow module on the backend and at the terminal definition (models and views) level on the frontend. Make the code and interfaces more consistent in my opinion and is more compatible with future directions I'd like to take things with mix-mode inputs/outputs for CWL.
Things not matching should be handled there.
- Fixes and refactoring for existing input tests (toward reuse). - Add simple test for non-data input.
80c4e4d to
4b574bf
Compare
4b574bf to
f12e548
Compare
|
Alright, this runs now, but there are some connectivity issues in the workflow editor. For instance if you toggle runtime inputs connections are not destroyed, and occasionally the input terminal either doesn't show up or isn't being destroyed properly. I guess this is probably one and the same issue, I'll have a look if I can figure this out. |
|
Ah, that's actually working fine. I just didn't fully realise that we have separate buttons for setting a Parameter at runtime and connecting a Parameter, sorry about that. Maybe we should have a text that explains this when you hover over the buttons ? |
mvdbeek
left a comment
There was a problem hiding this comment.
Fantastic stuff, thanks a lot @jmchilton !
|
Test failure is unrelated, I'll open a fix later. |
|
This is pretty cool. Thanks for the contribution @jmchilton. Sorry for the late review but I noticed a few minor issues. Clicking on a tool parameter label in the run workflow form does not collapse the input parameters anymore although the 'hand' cursor is shown. Another minor issue is that the collapsible and connected icons in the workfow editor are very close to each other and share the same tooltip which makes it difficult to select the correct one. |
PR galaxyproject#6925 introduced a GUI for connecting non-data (e.g. integer, boolean, color, etc..) workflow input parameter to tool input parameters (the backend for this was originally added in galaxyproject#1306). That ideally was just the beginning of work toward using such values in structured ways in workflows. This PR extends tool output handling to allow producing of non-data parameters. These can serve as a source for non-data values in workflows the same work workflow input parameters can. To make such values more easy to produce, this PR also introduces Galaxy expression tools - mirroring functionality regularly used in CWL. These small JavaScript-based tools that consume inputs just like a regular Galaxy tool but that produce dictionary of non-data values. I think these expressions will be maximally useful when paired with format 2 workflows once we allow users to load arbitrary tools (I make the case more in full here galaxyproject#7545 (comment)), but I outline some potential uses there as well. Because there is always a checklist in my PR descriptions: - Tool definition language and plumbing and datatype for expressing expressions as jobs. - Allow connecting expression tools to parameters in workflows, will delay evaluation of workflow so calculated value - Example test expression tools for testing and demonstration.
PR galaxyproject#6925 introduced a GUI for connecting non-data (e.g. integer, boolean, color, etc..) workflow input parameter to tool input parameters (the backend for this was originally added in galaxyproject#1306). That ideally was just the beginning of work toward using such values in structured ways in workflows. This PR extends tool output handling to allow producing of non-data parameters. These can serve as a source for non-data values in workflows the same work workflow input parameters can. To make such values more easy to produce, this PR also introduces Galaxy expression tools - mirroring functionality regularly used in CWL. These small JavaScript-based tools that consume inputs just like a regular Galaxy tool but that produce dictionary of non-data values. I think these expressions will be maximally useful when paired with format 2 workflows once we allow users to load arbitrary tools (I make the case more in full here galaxyproject#7545 (comment)), but I outline some potential uses there as well. Because there is always a checklist in my PR descriptions: - Tool definition language and plumbing and datatype for expressing expressions as jobs. - Allow connecting expression tools to parameters in workflows, will delay evaluation of workflow so calculated value - Example test expression tools for testing and demonstration.
An alternative to #6919, which was an alternative of #6829.
Differences from #6829:
Differences from #6919:
Brings in workflow API test definition improvements Allow API import/export of format 2 workflows. #6776 to define some new tests.(already merged into dev and this has been rebased)