Repository navigation
Draft: "method" task with multiple outputs #99
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
0fde785
5513a1c
e5e50cb
e425bd4
d821d3e
b3cd680
51af03c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| Python function as workflow task | ||
| ================================ | ||
|
|
||
| A Python function can be used as a task node in a workflow by using ``"method"`` as its ``task_type``. | ||
| By default, such task has a single output named ``return_value``. | ||
| It is possible to `Define multiple outputs`_. | ||
|
|
||
| Use a function as a task | ||
| ------------------------ | ||
|
|
||
| Example with a ``range_info`` function returning 3 values in a dictionary: | ||
|
|
||
| .. code:: python | ||
|
|
||
| def range_info(a: float, b: float): | ||
| return { | ||
| "extent": abs(b - a), | ||
| "minimum": min(a, b), | ||
| "maximum": max(a, b), | ||
| } | ||
|
|
||
| The corresponding workflow node must be declared with ``"method"`` as ``task_type``: | ||
|
|
||
| .. code:: python | ||
|
|
||
| range_info_node = { | ||
| "id": "task_range_info", | ||
| "task_type": "method", | ||
| "task_identifier": "__main__.range_info", | ||
| } | ||
|
|
||
| Code to execute the ``range_info`` function as a task in a workflow with ``a=15`` and ``b=10`` as inputs: | ||
|
|
||
| .. code:: python | ||
|
|
||
| from ewokscore import execute_graph | ||
|
|
||
| # Define a workflow which calls the range_info function as a task | ||
| workflow = { | ||
| "graph": {"id": "range_info_workflow"}, | ||
| "nodes": [ | ||
| { | ||
| "id": "task_range_info", | ||
| "task_type": "method", | ||
| "task_identifier": "__main__.range_info", | ||
| }, | ||
| ], | ||
| "links": [], | ||
| } | ||
|
|
||
| # Define task inputs | ||
| inputs = [ | ||
| {"id": "task_range_info", "name": "a", "value": 15}, | ||
| {"id": "task_range_info", "name": "b", "value": 10}, | ||
| ] | ||
|
|
||
| # Execute the workflow | ||
| result = execute_graph(workflow, inputs=inputs) | ||
| print(result) | ||
|
|
||
| The task output contains a single ``return_value`` field which is set to the function return value. | ||
|
|
||
| In the ``range_info`` example, the result contains a single output field ``return_value``: | ||
|
|
||
| .. code:: python | ||
|
|
||
| {'return_value': {'extent': 5, 'minimum': 10, 'maximum': 15}} | ||
|
|
||
|
|
||
| Define multiple outputs | ||
| ----------------------- | ||
|
|
||
| To declare a function that can be used as a :class:`Task` with multiple output fields: | ||
|
|
||
| * Declare the function with one of the following kind of return type: | ||
|
|
||
| .. tab-set:: | ||
|
|
||
| .. tab-item:: namedtuple | ||
|
|
||
| .. code:: python | ||
|
|
||
| from collections import namedtuple | ||
|
|
||
| Result = namedtuple("Result", ["extent", "minimum", "maximum"]) | ||
|
|
||
| .. tab-item:: dataclass | ||
|
|
||
| .. code:: python | ||
|
|
||
| from dataclasses import dataclass | ||
|
|
||
| @dataclass | ||
| class Result: | ||
| extent: float | ||
| minimum: float | ||
| maximum: float | ||
|
|
||
| .. tab-item:: typing.NamedTuple | ||
|
|
||
| .. code:: python | ||
|
|
||
| from typing import NamedTuple | ||
|
|
||
| class Result(NamedTuple): | ||
| extent: float | ||
| minimum: float | ||
| maximum: float | ||
|
|
||
| .. tab-item:: typing.TypedDict | ||
|
|
||
| .. code:: python | ||
|
|
||
| from typing import TypedDict | ||
|
|
||
| class Result(TypedDict): | ||
| extent: float | ||
| minimum: float | ||
| maximum: float | ||
|
|
||
| .. code:: python | ||
|
|
||
| def range_info(a: float, b: float) -> Result: | ||
|
|
||
| * Add the :func:`task_outputs` decorator to the function declaration: | ||
|
|
||
| .. code:: python | ||
|
|
||
| from ewokscore.methodtask import task_outputs | ||
|
|
||
| @task_outputs | ||
| def range_info(a: float, b: float) -> Result: | ||
| return Result( | ||
| extent=abs(b - a), | ||
| minimum=min(a, b), | ||
| maximum=max(a, b), | ||
| ) | ||
|
|
||
|
|
||
| When a ``@task_outputs`` decorated function is used as a :class:`Task`, the task output names are defined by the function return type. | ||
|
|
||
| In the example, the return value of ``range_info`` function is available through 3 task outputs: | ||
|
|
||
| .. code:: python | ||
|
|
||
| {'maximum': 15, 'extent': 5, 'minimum': 10} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,109 @@ | ||
| from .task import Task | ||
| from collections.abc import Mapping | ||
| import functools | ||
| import inspect | ||
| from types import FunctionType | ||
| from typing import get_type_hints, Set | ||
|
|
||
| from ewoksutils.import_utils import import_method | ||
|
|
||
| METHOD_ARGUMENT = "_method" | ||
| from .task import Task | ||
| from .utils import is_namedtuple, method_arguments | ||
|
|
||
|
|
||
| def task_outputs(function: FunctionType) -> FunctionType: | ||
| """Function decorator so ewoks extract task outputs from return type elements/attributes""" | ||
| # Report error early | ||
| if not _method_output_names(function): | ||
| raise ValueError("Function return type does not define any output name") | ||
|
|
||
| function._ewoks_unpack_outputs = True # noqa | ||
| return function | ||
|
|
||
|
|
||
| def _method_output_names(method) -> Set[str]: | ||
| sig = inspect.signature(method) | ||
| return_type = sig.return_annotation | ||
| if return_type is inspect.Signature.empty or not inspect.isclass(return_type): | ||
| return set() | ||
|
|
||
| return_annotations = get_type_hints(return_type) | ||
| if return_annotations: | ||
| return set(return_annotations.keys()) | ||
|
|
||
| if is_namedtuple(return_type): | ||
| return set(return_type._fields) | ||
|
|
||
| return set() | ||
|
|
||
|
|
||
| class MethodExecutorTask(Task): | ||
| _METHOD_REQUIRED_INPUTS = tuple() | ||
| _METHOD_OPTIONAL_INPUTS = tuple() | ||
| _METHOD_UNPACK_OUTPUTS = False | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In GitLab by @woutdenolf on Dec 16, 2024, 16:42 GMT+1: Ok it is because of |
||
|
|
||
| class MethodExecutorTask( | ||
| Task, input_names=[METHOD_ARGUMENT], output_names=["return_value"] | ||
| ): | ||
| METHOD_ARGUMENT = METHOD_ARGUMENT | ||
| def __init_subclass__(cls, task_identifier: str): | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In GitLab by @woutdenolf on Dec 16, 2024, 16:53 GMT+1: pass |
||
| cls._TASK_IDENTIFIER = task_identifier | ||
|
|
||
| method = import_method(task_identifier) | ||
| cls._METHOD_REQUIRED_INPUTS, cls._METHOD_OPTIONAL_INPUTS = method_arguments( | ||
| method | ||
| ) | ||
|
|
||
| cls._METHOD_UNPACK_OUTPUTS = getattr(method, "_ewoks_unpack_outputs", False) | ||
| if cls._METHOD_UNPACK_OUTPUTS: | ||
| output_names = _method_output_names(method) | ||
| if not output_names: | ||
| raise RuntimeError( | ||
| f"{task_identifier}'s return type do not define any output name" | ||
| ) | ||
| else: | ||
| output_names = ["return_value"] | ||
|
|
||
| super().__init_subclass__( | ||
| # required inputs are optional to support passing them as positional inputs | ||
| optional_input_names=cls._METHOD_REQUIRED_INPUTS | ||
| + cls._METHOD_OPTIONAL_INPUTS, | ||
| output_names=output_names, | ||
| ) | ||
| cls.__doc__ = inspect.getdoc(method) | ||
|
|
||
| # Advertise effective method required/optional input names | ||
| # rather than the less constrained task ones | ||
|
|
||
| @classmethod | ||
| def required_input_names(cls): | ||
| return cls._METHOD_REQUIRED_INPUTS | ||
|
|
||
| @classmethod | ||
| def optional_input_names(cls): | ||
| return cls._METHOD_OPTIONAL_INPUTS | ||
|
|
||
| def run(self): | ||
| kwargs = self.get_named_input_values() | ||
| args = self.get_positional_input_values() | ||
| fullname = kwargs.pop(self.METHOD_ARGUMENT) | ||
| method = import_method(fullname) | ||
|
|
||
| method = import_method(self._TASK_IDENTIFIER) | ||
|
|
||
| result = method(*args, **kwargs) | ||
|
|
||
| self.outputs.return_value = result | ||
| if not self._METHOD_UNPACK_OUTPUTS: | ||
| self.outputs.return_value = result | ||
| return | ||
|
|
||
| if isinstance(result, Mapping): | ||
| for name in self.output_names(): | ||
| if name in result: | ||
| self.outputs[name] = result[name] | ||
| return | ||
|
|
||
| for name in self.output_names(): | ||
| if hasattr(result, name): | ||
| self.outputs[name] = getattr(result, name) | ||
|
|
||
|
|
||
| @functools.lru_cache() | ||
| def get_method_task(task_identifier: str) -> MethodExecutorTask: | ||
| class MethodTask(MethodExecutorTask, task_identifier=task_identifier): | ||
| pass | ||
|
|
||
| return MethodTask | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,6 +125,8 @@ def instantiate( | |
| raise ValueError(f"Data proxy scheme '{scheme}' is not supported") | ||
|
|
||
| def serialize(self) -> Dict[str, str]: | ||
| if self.uri is None: | ||
| return None | ||
| return self.uri.serialize() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In GitLab by @tvincent on Dec 16, 2024, 15:32 GMT+1: Tests broke here, I didn't fully understand why...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| @classmethod | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In GitLab by @woutdenolf on Dec 16, 2024, 16:44 GMT+1:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In GitLab by @tvincent on Dec 16, 2024, 16:51 GMT+1:
changed this line in version 2 of the diff