Skip to content

[WIP] Implement conditionals. #123

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 15 commits into from
Closed

[WIP] Implement conditionals. #123

wants to merge 15 commits into from

Conversation

jmchilton
Copy link 

sh run_tests.sh -api lib/galaxy_test/api/cwl/test_cwl_conformance_red_v1_2.py::CwlConformanceTestCase::test_conformance_v1_2_direct_optional_null_result

@jmchilton jmchilton mentioned this pull request Nov 9, 2020
Closed
49 tasks
@jmchilton jmchilton force-pushed the cwl-conditionals branch 3 times, most recently from 4259b6e to 03c5606 Compare November 10, 2020 04:38
@jmchilton
Copy link
Author

03c5606 is the actual conditional implementing, and I think it passes every test case that includes a when with no pickValues!

I was remembering pickValues being on inputs... but I was clearly wrong. All the test cases have them on workflow outputs and that is the only place they can be according to the spec. This makes implementing them more tricky for sure - some ideas for paths forward on #126

mvdbeek
mvdbeek reviewed Nov 11, 2020
View reviewed changes
lib/galaxy/workflow/modules.py
@@ -1760,6 +1761,23 @@ def from_cwl(value):
for key, value in execution_state.inputs.items():
step_state[key] = to_cwl(value)

if when_expression is not None:
from cwltool.expression import do_eval
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's import this at the top

Copy link
Author

Choose a reason for hiding this comment

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

You can tell I have developed habits stemming from frequent rebasing, imports at top result in conflicts 😆 .

@jmchilton jmchilton force-pushed the cwl-1.0 branch 2 times, most recently from 05e650a to 177702b Compare November 13, 2020 16:51
@mr-c
Copy link
Member

mr-c commented Nov 15, 2020

Hey @jmchilton , what can we do to get this merged?

@mr-c
Copy link
Member

mr-c commented Nov 18, 2020

I guess this PR can get rebased or merged with galaxyproject#10712 now that it has been accepted upstream?

@nsoranzo nsoranzo force-pushed the cwl-1.0 branch 12 times, most recently from e5d6948 to 7f21c69 Compare January 4, 2021 11:26
@nsoranzo nsoranzo force-pushed the cwl-1.0 branch 2 times, most recently from 4b69370 to f19ccf9 Compare September 20, 2022 08:59
@nsoranzo nsoranzo force-pushed the cwl-1.0 branch 4 times, most recently from b85744d to e2636c2 Compare September 29, 2022 01:21
@nsoranzo nsoranzo force-pushed the cwl-1.0 branch 3 times, most recently from 9e3eb29 to b6ff70d Compare October 17, 2022 13:28
@nsoranzo nsoranzo force-pushed the cwl-1.0 branch 10 times, most recently from e90dbbd to 3c57abe Compare November 11, 2022 01:12
@nsoranzo nsoranzo force-pushed the cwl-1.0 branch 2 times, most recently from 2857bf9 to a22fdcf Compare December 1, 2022 01:16
@nsoranzo nsoranzo force-pushed the cwl-1.0 branch 3 times, most recently from 8009556 to 8488047 Compare January 17, 2023 15:48
@mvdbeek mvdbeek force-pushed the cwl-1.0 branch 4 times, most recently from 9a13f5f to ef9f5d4 Compare October 31, 2023 14:28
@mr-c
Copy link
Member

mr-c commented Nov 1, 2023

Already merged!

@mr-c mr-c closed this Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvdbeek mvdbeek mvdbeek left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jmchilton @mr-c @mvdbeek @nsoranzo