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

Clarify input behaviour #27

Merged
merged 9 commits into from
Jun 22, 2024

Conversation

philtweir
Copy link
Contributor

@philtweir philtweir commented May 16, 2024

What does this PR do?

The fundamental problem it solves is that, as per feedback, when a raw value is passed as an argument to a function, it should not simply be captured as a default value for that function call, but be turned into a global parameter. However, this behaviour does not make sense for raw (literal) variables inside tasks/nested tasks. In practice, this turned out to be quite a major change with a number of corner cases.

Consequently, this PR includes the following:

  • subworkflows: nested tasks that are not transparent to the outside workflow, effectively having a scope and preventing literal values in nested tasks (when marked as "subworkflows" from bubbling up to the global parameters).
  • parameter-naming: unnamed parameters are given names that should be predictable, but will remain distinct from each other. A potentially undesirable sideeffect is that two tasks called with the same raw variable (i.e. one not declared as a parameter) will always have separate defaults, even if they were originally the same static variable. There does not appear to be a straightforward (non-convoluted/brittle) way around this.
  • the renderers can return multiple (related) workflows if subworkflows are discovered.

How to test?

Run through the documented workflows in the docs folder, and ensure they make sense with the input code.

Who can review?

@elleryames or @KamenDimitrov97

@philtweir philtweir changed the base branch from main to release/0.9.1 May 29, 2024 11:19
@philtweir philtweir force-pushed the feat/26-clarify-feedback-input-1-16-1-17 branch from 3690794 to 5e76712 Compare June 3, 2024 18:30
@philtweir philtweir force-pushed the feat/26-clarify-feedback-input-1-16-1-17 branch from 5e76712 to 3b5a38b Compare June 3, 2024 18:32
@philtweir philtweir marked this pull request as ready for review June 6, 2024 12:39
@elleryames
Copy link
Contributor

elleryames commented Jun 10, 2024

The import of nested_task in workflows.md line 346 is not needed for the (shuffle) example.

@elleryames
Copy link
Contributor

I understand that this PR introduces a new concept of subworkflows, and there is test for this feature in test_cwl.py. We should in addition document and give an example for this feature, perhaps in workflows.md.

@philtweir philtweir merged commit 80d4b0b into release/0.9.1 Jun 22, 2024
12 checks passed
@philtweir philtweir changed the title [Requires #25] Clarify input behaviour Clarify input behaviour Jun 22, 2024
@KamenDimitrov97 KamenDimitrov97 mentioned this pull request Aug 7, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants