Draft: "method" task with multiple outputs#99
Conversation
| def serialize(self) -> Dict[str, str]: | ||
| if self.uri is None: | ||
| return None | ||
| return self.uri.serialize() |
There was a problem hiding this comment.
In GitLab by @tvincent on Dec 16, 2024, 15:32 GMT+1:
Tests broke here, I didn't fully understand why...
There was a problem hiding this comment.
In GitLab by @woutdenolf on Dec 16, 2024, 17:38 GMT+1:
This is caused by hacking the inputs to support the same argument to be positional and named.
If we want to support this, we need to support it on a fundamental level, this cannot be patched.
There was a problem hiding this comment.
In GitLab by @tvincent on Dec 16, 2024, 17:50 GMT+1:
OK, then I propose to support named only (and eventually positional only) = A single way to pass arguments.
This would also avoids the required/optional input names workaround I had to do, but would change the current behavior.
There was a problem hiding this comment.
In GitLab by @woutdenolf on Dec 17, 2024, 09:59 GMT+1:
We can support it, it just needs to be native support where a variable can have two id's (a string and an integer). But it is outside the scope of this MR. So named-only would be best.
| class MethodExecutorTask(Task): | ||
| _METHOD_REQUIRED_INPUTS = tuple() | ||
| _METHOD_OPTIONAL_INPUTS = tuple() | ||
| _METHOD_UNPACK_OUTPUTS = False |
There was a problem hiding this comment.
In GitLab by @woutdenolf on Dec 16, 2024, 16:40 GMT+1:
The base class already has
_INPUT_NAMES = set()
_OPTIONAL_INPUT_NAMES = set()
_OUTPUT_NAMES = set()
_N_REQUIRED_POSITIONAL_INPUTS = 0Why no use those?
There was a problem hiding this comment.
In GitLab by @woutdenolf on Dec 16, 2024, 16:42 GMT+1:
Ok it is because of required inputs are optional to support passing them as positional inputs
|
|
||
|
|
||
| def task_outputs(function: FunctionType) -> FunctionType: | ||
| """Function decorator so ewoks extract task outputs from return type elements/attributes""" |
There was a problem hiding this comment.
In GitLab by @woutdenolf on Dec 16, 2024, 16:44 GMT+1:
"""Function decorator so ewoks extracts task outputs from return type elements/attributes"""
There was a problem hiding this comment.
In GitLab by @tvincent on Dec 16, 2024, 16:51 GMT+1:
changed this line in version 2 of the diff
| Task, input_names=[METHOD_ARGUMENT], output_names=["return_value"] | ||
| ): | ||
| METHOD_ARGUMENT = METHOD_ARGUMENT | ||
| def __init_subclass__(cls, task_identifier: str): |
There was a problem hiding this comment.
In GitLab by @woutdenolf on Dec 16, 2024, 16:53 GMT+1:
pass **kwargs to super().__init_subclass__
poautran
force-pushed
the
main
branch
2 times, most recently
from
8dcd50a to
b58fb90
Compare
2 participants
In GitLab by @tvincent on Dec 16, 2024, 15:31 GMT+1:
This MR proposes a way to define a Task with multiple outputs from a Python function.
For me the goal is two-fold:
I can imagine different ways to do this, this MR proposes one approach: Use the function return type annotation to define the task output fields.
As it is, by default there is a single "return_value" output and the use of return type is enabled through a decorator.
I'm not sure about the best default and the way to choose to "unpack" outputs or keep a single one, but I'd expect allowing both is needed.
Related to #46
Migrated from GitLab: https://gitlab.esrf.fr/workflow/ewoks/ewokscore/-/merge_requests/262