Skip to content

Handle-none-outputs #790

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/source/tutorial/2-advanced-execution.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,8 @@
"how to utilise containers and add support for other software environments.\n",
"\n",
"It is also possible to specify functions to run at hooks that are immediately before and after\n",
"the task is executed by passing a `pydra.engine.spec.TaskHooks` object to the `hooks`\n",
"keyword arg. The callable should take the `pydra.engine.core.Job` object as its only\n",
"the task is executed by passing a `pydra.engine.hooks.TaskHooks` object to the `hooks`\n",
"keyword arg. The callable should take the `pydra.engine.job.Job` object as its only\n",
"argument and return None. The available hooks to attach functions are:\n",
"\n",
"* pre_run: before the task cache directory is created\n",
Expand Down Expand Up @@ -415,7 +415,7 @@
],
"metadata": {
"kernelspec": {
"display_name": "wf12",
"display_name": "wf13",
"language": "python",
"name": "python3"
},
Expand All @@ -429,7 +429,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.5"
"version": "3.13.1"
}
},
"nbformat": 4,
Expand Down
4 changes: 2 additions & 2 deletions docs/source/tutorial/7-canonical-form.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
"Default values can also be set directly, as with Attrs classes.\n",
"\n",
"In order to allow static type-checkers to check the type of outputs of tasks added\n",
"to workflows, it is also necessary to explicitly extend from the `pydra.engine.python.Task`\n",
"and `pydra.engine.python.Outputs` classes (they are otherwise set as bases by the\n",
"to workflows, it is also necessary to explicitly extend from the `pydra.compose.python.Task`\n",
"and `pydra.compose.python.Outputs` classes (they are otherwise set as bases by the\n",
"`define` method implicitly). Thus the \"canonical form\" of Python task is as\n",
"follows"
]
Expand Down
72 changes: 37 additions & 35 deletions pydra/compose/base/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@
input_types[p.name] = type_hints.get(p.name, ty.Any)
if p.default is not inspect.Parameter.empty:
input_defaults[p.name] = p.default
if inputs:
if inputs is not None:
if not isinstance(inputs, dict):
raise ValueError(
f"Input names ({inputs}) should not be provided when "
Comment on lines +186 to 189
Copy link
Contributor

Choose a reason for hiding this comment

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

Just looking at this, it seems like inputs could be a list[str | Arg], but it isn't turned into a dict before this. This means we'll now error on an empty list as well as a populated list, but maybe we should just remove the list annotation?

Expand Down Expand Up @@ -218,41 +218,43 @@
f"value {default}"
)
return_type = type_hints.get("return", ty.Any)
if outputs and len(outputs) > 1:
if return_type is not ty.Any:
if ty.get_origin(return_type) is not tuple:
raise ValueError(
f"Multiple outputs specified ({outputs}) but non-tuple "
f"return value {return_type}"
)
return_types = ty.get_args(return_type)
if len(return_types) != len(outputs):
raise ValueError(
f"Length of the outputs ({outputs}) does not match that "
f"of the return types ({return_types})"
)
output_types = dict(zip(outputs, return_types))
else:
output_types = {o: ty.Any for o in outputs}
if isinstance(outputs, dict):
for output_name, output in outputs.items():
if isinstance(output, Out) and output.type is ty.Any:
output.type = output_types[output_name]
if outputs:
if len(outputs) > 1:
if return_type is not ty.Any:
if ty.get_origin(return_type) is not tuple:
raise ValueError(
f"Multiple outputs specified ({outputs}) but non-tuple "
f"return value {return_type}"
)
return_types = ty.get_args(return_type)
if len(return_types) != len(outputs):
raise ValueError(
f"Length of the outputs ({outputs}) does not match that "
f"of the return types ({return_types})"
)
output_types = dict(zip(outputs, return_types))
else:
output_types = {o: ty.Any for o in outputs}
if isinstance(outputs, dict):
for output_name, output in outputs.items():
if isinstance(output, Out) and output.type is ty.Any:
output.type = output_types[output_name]

Check warning on line 241 in pydra/compose/base/helpers.py

View check run for this annotation

Codecov / codecov/patch

pydra/compose/base/helpers.py#L241

Added line #L241 was not covered by tests
else:
outputs = output_types
else:
outputs = output_types

elif outputs:
if isinstance(outputs, dict):
output_name, output = next(iter(outputs.items()))
elif isinstance(outputs, list):
output_name = outputs[0]
output = ty.Any
if isinstance(output, Out):
if output.type is ty.Any:
output.type = return_type
elif output is ty.Any:
output = return_type
outputs = {output_name: output}
if isinstance(outputs, dict):
output_name, output = next(iter(outputs.items()))
elif isinstance(outputs, list):
output_name = outputs[0]
output = ty.Any
if isinstance(output, Out):
if output.type is ty.Any:
output.type = return_type

Check warning on line 252 in pydra/compose/base/helpers.py

View check run for this annotation

Codecov / codecov/patch

pydra/compose/base/helpers.py#L252

Added line #L252 was not covered by tests
elif output is ty.Any:
output = return_type
outputs = {output_name: output}
elif outputs == [] or return_type in (None, type(None)):
outputs = {}
else:
outputs = {"out": return_type}
return inputs, outputs
Expand Down
4 changes: 4 additions & 0 deletions pydra/compose/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,10 @@ def _run(self, job: "Job[PythonTask]", rerun: bool = True) -> None:
return_names = [f.name for f in task_fields(self.Outputs)]
if returned is None:
job.return_values = {nm: None for nm in return_names}
elif not return_names:
raise ValueError(
f"No output fields were specified, but the function returned {returned}"
)
elif len(return_names) == 1:
# if only one element in the fields, everything should be returned together
job.return_values[return_names[0]] = returned
Expand Down
51 changes: 51 additions & 0 deletions pydra/compose/tests/test_python_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -424,3 +424,54 @@

outputs = TestFunc(a=A(x=7))()
assert outputs.out == 7


def test_no_outputs1():
"""Test function tasks with no outputs specified by None return type"""

@python.define
def TestFunc1(a: A) -> None:
print(a)

outputs = TestFunc1(a=A(x=7))()
assert not task_fields(outputs)


def test_no_outputs2():
"""Test function tasks with no outputs set explicitly"""

@python.define(outputs=[])
def TestFunc2(a: A):
print(a)

Check warning on line 445 in pydra/compose/tests/test_python_fields.py

View check run for this annotation

Codecov / codecov/patch

pydra/compose/tests/test_python_fields.py#L445

Added line #L445 was not covered by tests

outputs = TestFunc2(a=A(x=7))()
assert not task_fields(outputs)


def test_no_outputs_fail():
"""Test function tasks with object inputs"""

@python.define(outputs=[])
def TestFunc3(a: A):
return a

with pytest.raises(ValueError, match="No output fields were specified"):
TestFunc3(a=A(x=7))()


def test_only_one_output_fail():

with pytest.raises(ValueError, match="Multiple outputs specified"):

@python.define(outputs=["out1", "out2"])
def TestFunc4(a: A) -> A:
return a

Check warning on line 468 in pydra/compose/tests/test_python_fields.py

View check run for this annotation

Codecov / codecov/patch

pydra/compose/tests/test_python_fields.py#L468

Added line #L468 was not covered by tests


def test_incorrect_num_outputs_fail():

with pytest.raises(ValueError, match="Length of the outputs"):

@python.define(outputs=["out1", "out2"])
def TestFunc5(a: A) -> tuple[A, A, A]:
return a, a, a

Check warning on line 477 in pydra/compose/tests/test_python_fields.py

View check run for this annotation

Codecov / codecov/patch

pydra/compose/tests/test_python_fields.py#L477

Added line #L477 was not covered by tests
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ testpaths = ["pydra"]
log_cli_level = "INFO"
xfail_strict = true
addopts = [
"-svx",
"-svv",
"-ra",
"--strict-config",
"--strict-markers",
Expand Down
Loading