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

Implementation of proposed syntax changes #768

Open
wants to merge 345 commits into
base: master
Choose a base branch
from
Open

Implementation of proposed syntax changes #768

wants to merge 345 commits into from

Conversation

tclose
Copy link
Contributor

@tclose tclose commented Feb 17, 2025

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary

Implements proposed syntax changes in #692

Test modules (to fix)

  • test_python (design)
  • test_shell (design)
  • test_workflow (design)
  • test_boutiques
  • test_dockertask
  • test_environments
  • test_functions
  • test_graph
  • test_helpers_file
  • test_helpers_state
  • test_helpers
  • test_node_task
  • test_numpy_examples
  • test_profiles
  • test_shelltask_inputspec
  • test_shelltask
  • test_singularity
  • test_specs
  • test_state
  • test_submitter
  • test_task
  • test_tasks_files
  • test_workflow (engine)
  • test_hash
  • test_messenger
  • test_typing

Checklist

  • I have added tests to cover my changes (if necessary)
  • I have updated documentation (if necessary)

List of allowed values for the field.
xor: Sequence[str | None], optional
Names of args that are exclusive mutually exclusive, which must include
the name of the current field. If this list includes None, then none of the
Copy link
Contributor

Choose a reason for hiding this comment

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

including the current field was not the case in nipype or the previous version of pydra i believe. reducing this kind of boilerplate would be helpful, and also if xor is commutative without specification (a xor b implies b xor a and should not have to be specified in both places - just one should do the other should be computed). also any checks to make sure xor combinations are valid or invalid (e.g. a xor b, b xor c, c xor a)

Copy link
Contributor Author

@tclose tclose Mar 6, 2025

Choose a reason for hiding this comment

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

I was thinking it would be better as an argument to the shell.define decorator/function, so it would only need to be defined once, and it is really a "task-level" attribute

Copy link
Contributor

Choose a reason for hiding this comment

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

so a list of lists? i.e. there could be multiple different xors in a task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, maybe list[str] | list[list[str]] could be acceptable inputs

tclose added 28 commits March 6, 2025 13:34
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.

3 participants