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

Support for Enhancements to gxformat2. #6807

Merged
merged 4 commits into from Oct 7, 2018
Merged

Conversation

@jmchilton
Copy link
Member

@jmchilton jmchilton commented Sep 30, 2018

Builds on #6746, to provide even better support for Galaxy Format 2 workflows.

A few changes:

  • Import the gxformat2 module as an external dependency instead of using code from within the test/ directory. This isn't strictly needed yet but will be in subsequent PRs (#6776) that bring Format 2 support into Galaxy core directly. I had initially done this by moving the code into lib/ but I decided it would be better to maintain one canonical source and skip the complexity of sync'ing stuff the way we do with galaxy-lib.
  • Revs the version of gxformat2 which contains some fixes, but most importantly adds a CWL-style input connection handling with an in keyword. A minor syntax difference but it should go a long way toward making gxformat2 workflows - proper CWL workflows with extensions for Galaxy style tools.

in requires skipping the special special link syntax with tool state. The embedded state syntax using $link still works fine though but it is very particular to tool steps.

   - tool_id: cat1
     label: first_cat
-    state:
-      input1:
-        $link: outer_input
+    in:
+      input1: outer_input

For non-tool steps though the in syntax looks a lot like the connect syntax which already existed. Just using / to separate step ID and output name.

     label: nested_workflow
-    connect:
-      inner_input: first_cat#out_file1
+    in:
+      inner_input: first_cat/out_file1

All three syntaxes are still supported, I suppose at very least we should say in is preferred to connect going forward and drop support for connect as a keyword.

  • Finally, Galaxy workflows will work if connections are references in conditionals and at the top level but don't have empty tool state values - but within repeats if there wasn't an empty repeat block in the tool state and an input connection was specified that would require a repeat instance - that wouldn't work because the repeat instance that is required wouldn't be visited by the tool parameter visit code. This tweaks workflow import to create empty repeat instances needed for input connections that are specified without empty tool state blocks. There is a long discussion in the doc strings on c685b6b and a test case. The tl;dr is that this makes it feasible to define steps with just connections (in statements) and not require state statements for tools.
@mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Oct 6, 2018

This is conflicting now that I merged #6746, but it does look good to me, I think the syntax is a lot cleaner, and moving gxformat2 to an external dep seems like the right thing to do. Maybe we could have a more descriptive package name though, like galaxy-workflow-lib, lib-galaxy-workflow or something like that ?

Loading

jmchilton added 4 commits Oct 6, 2018
Makes gxformat2 look a lot closer to CWL workflows with Galaxy tools and state.
…atching empty tool state entries.

There is a long conversation in the added doc string.
@mvdbeek mvdbeek merged commit 7d9093d into galaxyproject:dev Oct 7, 2018
6 checks passed
Loading
@nsoranzo nsoranzo deleted the gxformat2_030 branch Oct 8, 2018
@nsoranzo
Copy link
Member

@nsoranzo nsoranzo commented Oct 8, 2018

@jmchilton This is breaking selenium tests on dev: https://jenkins.galaxyproject.org/job/docker-selenium/3811/console

$ ack workflows_format_2
test/manual/workflows_scaling.py
27:from base.workflows_format_2.converter import python_to_workflow  # noqa: I100

test/selenium_tests/framework.py
23:from base.workflows_format_2 import (  # noqa: I100

Loading

@jmchilton
Copy link
Member Author

@jmchilton jmchilton commented Oct 8, 2018

I see - thanks @nsoranzo - I'll get a fix right in.

Loading

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

Successfully merging this pull request may close these issues.

None yet

4 participants