From 4da2eefaff7dc9170c6b4df58db783ad56a1b086 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Wed, 29 Jan 2025 19:21:33 +1100 Subject: [PATCH] debugging unittests --- new-docs/source/tutorial/5-shell.ipynb | 10 +- new-docs/tst.py | 4 +- pydra/design/base.py | 23 +-- pydra/design/python.py | 7 +- pydra/design/shell.py | 31 ++-- pydra/design/tests/test_shell.py | 192 +++++++++++---------- pydra/engine/helpers.py | 4 +- pydra/engine/specs.py | 36 ++-- pydra/engine/tests/test_dockertask.py | 30 ++-- pydra/engine/tests/test_environments.py | 4 +- pydra/engine/tests/test_nipype1_convert.py | 2 +- pydra/engine/tests/test_shelltask.py | 10 +- pydra/engine/tests/test_specs.py | 4 +- pydra/engine/tests/test_submitter.py | 8 +- pydra/engine/tests/test_task.py | 70 ++++---- pydra/engine/tests/test_workflow.py | 20 +-- pydra/utils/tests/test_typing.py | 8 +- 17 files changed, 251 insertions(+), 212 deletions(-) diff --git a/new-docs/source/tutorial/5-shell.ipynb b/new-docs/source/tutorial/5-shell.ipynb index 3b90c6488..0cb40bc08 100644 --- a/new-docs/source/tutorial/5-shell.ipynb +++ b/new-docs/source/tutorial/5-shell.ipynb @@ -67,11 +67,11 @@ "print(f\"Command-line to be run: {cp.cmdline}\")\n", "\n", "# Run the shell-comand task\n", - "result = cp()\n", + "outputs = cp()\n", "\n", "print(\n", - " f\"Contents of copied file ('{result.output.destination}'): \"\n", - " f\"'{Path(result.output.destination).read_text()}'\"\n", + " f\"Contents of copied file ('{outputs.destination}'): \"\n", + " f\"'{Path(outputs.destination).read_text()}'\"\n", ")" ] }, @@ -335,10 +335,10 @@ "cp_with_size = CpWithSize(in_file=File.sample())\n", "\n", "# Run the command\n", - "result = cp_with_size()\n", + "outputs = cp_with_size()\n", "\n", "\n", - "print(f\"Size of the output file is: {result.output.out_file_size}\")" + "print(f\"Size of the output file is: {outputs.out_file_size}\")" ] }, { diff --git a/new-docs/tst.py b/new-docs/tst.py index d3589ba07..461b89d79 100644 --- a/new-docs/tst.py +++ b/new-docs/tst.py @@ -8,7 +8,7 @@ load_json = LoadJson(file=json_file) # Run the task -result = load_json(plugin="serial") +outputs = load_json(plugin="serial") # Print the output interface of the of the task (LoadJson.Outputs) -print(result.outputs) +print(outputs) diff --git a/pydra/design/base.py b/pydra/design/base.py index f5dd8b750..ab7b376f8 100644 --- a/pydra/design/base.py +++ b/pydra/design/base.py @@ -54,7 +54,7 @@ def __bool__(self): def convert_default_value(value: ty.Any, self_: "Field") -> ty.Any: """Ensure the default value has been coerced into the correct type""" - if value is EMPTY: + if value is EMPTY or isinstance(value, attrs.Factory): return value return TypeParser[self_.type](self_.type, label=self_.name)(value) @@ -197,6 +197,10 @@ def requirements_satisfied(self, inputs: "TaskDef") -> bool: """Check if all the requirements are satisfied by the inputs""" return any(req.satisfied(inputs) for req in self.requires) + @property + def mandatory(self): + return self.default is EMPTY + @attrs.define(kw_only=True) class Arg(Field): @@ -240,7 +244,7 @@ class Arg(Field): readonly: bool = False -@attrs.define(kw_only=True, slots=False) +@attrs.define(kw_only=True) class Out(Field): """Base class for output fields of task definitions @@ -265,7 +269,7 @@ class Out(Field): The order of the output in the output list, allows for tuple unpacking of outputs """ - order: int = attrs.field(default=None) + pass def extract_fields_from_class( @@ -394,10 +398,6 @@ def make_task_def( spec_type._check_arg_refs(inputs, outputs) - # Set positions for outputs to allow for tuple unpacking - for i, output in enumerate(outputs.values()): - output.order = i - if name is None and klass is not None: name = klass.__name__ if reserved_names := [n for n in inputs if n in spec_type.RESERVED_FIELD_NAMES]: @@ -405,11 +405,11 @@ def make_task_def( f"{reserved_names} are reserved and cannot be used for {spec_type} field names" ) outputs_klass = make_outputs_spec(out_type, outputs, outputs_bases, name) + if issubclass(klass, TaskDef) and not issubclass(klass, spec_type): + raise ValueError(f"Cannot change type of definition {klass} to {spec_type}") if klass is None or not issubclass(klass, spec_type): if name is None: raise ValueError("name must be provided if klass is not") - if klass is not None and issubclass(klass, TaskDef): - raise ValueError(f"Cannot change type of definition {klass} to {spec_type}") bases = tuple(bases) # Ensure that TaskDef is a base class if not any(issubclass(b, spec_type) for b in bases): @@ -518,16 +518,17 @@ def make_outputs_spec( field.name = name field.type = base.__annotations__.get(name, ty.Any) outputs.update(base_outputs) + assert all(o.name == n for n, o in outputs.items()) outputs_klass = type( spec_name + "Outputs", tuple(outputs_bases), { - o.name: attrs.field( + n: attrs.field( converter=make_converter(o, f"{spec_name}.Outputs"), metadata={PYDRA_ATTR_METADATA: o}, **_get_default(o), ) - for o in outputs.values() + for n, o in outputs.items() }, ) outputs_klass.__annotations__.update((o.name, o.type) for o in outputs.values()) diff --git a/pydra/design/python.py b/pydra/design/python.py index 3baf7c598..8036e5bc1 100644 --- a/pydra/design/python.py +++ b/pydra/design/python.py @@ -80,7 +80,7 @@ class out(Out): outputs """ - pass + order: int = attrs.field(default=None) @dataclass_transform( @@ -161,6 +161,11 @@ def make(wrapped: ty.Callable | type) -> PythonDef: name="function", type=ty.Callable, default=function ) + # Set positions for outputs to allow for tuple unpacking + output: out + for i, output in enumerate(parsed_outputs.values()): + output.order = i + interface = make_task_def( PythonDef, PythonOutputs, diff --git a/pydra/design/shell.py b/pydra/design/shell.py index 3a51b9f5d..cafeb0b29 100644 --- a/pydra/design/shell.py +++ b/pydra/design/shell.py @@ -105,11 +105,12 @@ def _validate_sep(self, attribute, value): value is not None and self.type is not ty.Any and ty.get_origin(self.type) is not MultiInputObj - and not issubclass(self.type, ty.Iterable) ): - raise ValueError( - f"sep ({value!r}) can only be provided when type is iterable" - ) + tp = ty.get_origin(self.type) or self.type + if not issubclass(tp, ty.Iterable): + raise ValueError( + f"sep ({value!r}) can only be provided when type is iterable" + ) @attrs.define(kw_only=True) @@ -353,6 +354,12 @@ def make( if class_name[0].isdigit(): class_name = f"_{class_name}" + # Add in fields from base classes + parsed_inputs.update({n: getattr(ShellDef, n) for n in ShellDef.BASE_NAMES}) + parsed_outputs.update( + {n: getattr(ShellOutputs, n) for n in ShellOutputs.BASE_NAMES} + ) + # Update the inputs (overriding inputs from base classes) with the executable # and the output argument fields parsed_inputs.update( @@ -371,10 +378,12 @@ def make( # Set positions for the remaining inputs that don't have an explicit position position_stack = remaining_positions(list(parsed_inputs.values())) for inpt in parsed_inputs.values(): + if inpt.name == "additional_args": + continue if inpt.position is None: inpt.position = position_stack.pop(0) - interface = make_task_def( + defn = make_task_def( ShellDef, ShellOutputs, parsed_inputs, @@ -384,7 +393,7 @@ def make( bases=bases, outputs_bases=outputs_bases, ) - return interface + return defn # If a name is provided (and hence not being used as a decorator), check to see if # we are extending from a class that already defines an executable @@ -479,17 +488,19 @@ def parse_command_line_template( outputs = {} parts = template.split() executable = [] - for i, part in enumerate(parts, start=1): + start_args_index = 0 + for part in parts: if part.startswith("<") or part.startswith("-"): break executable.append(part) + start_args_index += 1 if not executable: raise ValueError(f"Found no executable in command line template: {template}") if len(executable) == 1: executable = executable[0] - if i == len(parts): + args_str = " ".join(parts[start_args_index:]) + if not args_str: return executable, inputs, outputs - args_str = " ".join(parts[i - 1 :]) tokens = re.split(r"\s+", args_str.strip()) arg_pattern = r"<([:a-zA-Z0-9_,\|\-\.\/\+]+(?:\?|=[^>]+)?)>" opt_pattern = r"--?[a-zA-Z0-9_]+" @@ -662,7 +673,7 @@ def remaining_positions( # Check for multiple positions positions = defaultdict(list) for arg in args: - if arg.name == "arguments": + if arg.name == "additional_args": continue if arg.position is not None: if arg.position >= 0: diff --git a/pydra/design/tests/test_shell.py b/pydra/design/tests/test_shell.py index bcbab495a..9a04eb2b6 100644 --- a/pydra/design/tests/test_shell.py +++ b/pydra/design/tests/test_shell.py @@ -35,11 +35,12 @@ def test_interface_template(): validator=attrs.validators.min_len(1), default="cp", type=str | ty.Sequence[str], - argpos=0, + position=0, help=shell.EXECUTABLE_HELP_STRING, ), - shell.arg(name="in_path", type=FsObject, argpos=1), + shell.arg(name="in_path", type=FsObject, position=1), output, + ShellDef.additional_args, ] assert sorted_fields(Cp.Outputs) == [ output, @@ -82,11 +83,12 @@ def test_interface_template_w_types_and_path_template_ext(): validator=attrs.validators.min_len(1), default="trim-png", type=str | ty.Sequence[str], - argpos=0, + position=0, help=shell.EXECUTABLE_HELP_STRING, ), - shell.arg(name="in_image", type=image.Png, argpos=1), + shell.arg(name="in_image", type=image.Png, position=1), output, + ShellDef.additional_args, ] assert sorted_fields(TrimPng.Outputs) == [ output, @@ -122,10 +124,13 @@ def test_interface_template_w_modify(): validator=attrs.validators.min_len(1), default="trim-png", type=str | ty.Sequence[str], - argpos=0, + position=0, help=shell.EXECUTABLE_HELP_STRING, ), - shell.arg(name="image", type=image.Png, argpos=1, copy_mode=File.CopyMode.copy), + shell.arg( + name="image", type=image.Png, position=1, copy_mode=File.CopyMode.copy + ), + ShellDef.additional_args, ] assert sorted_fields(TrimPng.Outputs) == [ shell.out( @@ -178,35 +183,36 @@ def test_interface_template_more_complex(): validator=attrs.validators.min_len(1), default="cp", type=str | ty.Sequence[str], - argpos=0, + position=0, help=shell.EXECUTABLE_HELP_STRING, ), shell.arg( - name="in_fs_objects", type=MultiInputObj[FsObject], argpos=1, sep=" " + name="in_fs_objects", type=MultiInputObj[FsObject], position=1, sep=" " ), output, - shell.arg(name="recursive", argstr="-R", type=bool, default=False, argpos=3), + shell.arg(name="recursive", argstr="-R", type=bool, default=False, position=3), shell.arg( name="text_arg", argstr="--text-arg", type=str | None, default=None, - argpos=4, + position=4, ), shell.arg( name="int_arg", argstr="--int-arg", type=int | None, default=None, - argpos=5, + position=5, ), shell.arg( name="tuple_arg", argstr="--tuple-arg", type=tuple[int, str] | None, default=None, - argpos=6, + position=6, ), + ShellDef.additional_args, ] assert sorted_fields(Cp.Outputs) == [ output, @@ -268,45 +274,41 @@ def test_interface_template_with_overrides_and_optionals(): position=-1, ), ] - assert ( - sorted_fields(Cp) - == [ - shell.arg( - name="executable", - validator=attrs.validators.min_len(1), - default="cp", - type=str | ty.Sequence[str], - argpos=0, - help=shell.EXECUTABLE_HELP_STRING, - ), - shell.arg( - name="in_fs_objects", type=MultiInputObj[FsObject], argpos=1, sep=" " - ), - shell.arg( - name="recursive", - argstr="-R", - type=bool, - default=False, - help=RECURSIVE_HELP, - argpos=2, - ), - shell.arg(name="text_arg", argstr="--text-arg", type=str, argpos=3), - shell.arg( - name="int_arg", - argstr="--int-arg", - type=int | None, - default=None, - argpos=4, - ), - shell.arg( - name="tuple_arg", - argstr="--tuple-arg", - type=tuple[int, str], - argpos=5, - ), - ] - + outargs - ) + assert sorted_fields(Cp) == [ + shell.arg( + name="executable", + validator=attrs.validators.min_len(1), + default="cp", + type=str | ty.Sequence[str], + position=0, + help=shell.EXECUTABLE_HELP_STRING, + ), + shell.arg( + name="in_fs_objects", type=MultiInputObj[FsObject], position=1, sep=" " + ), + shell.arg( + name="recursive", + argstr="-R", + type=bool, + default=False, + help=RECURSIVE_HELP, + position=2, + ), + shell.arg(name="text_arg", argstr="--text-arg", type=str, position=3), + shell.arg( + name="int_arg", + argstr="--int-arg", + type=int | None, + default=None, + position=4, + ), + shell.arg( + name="tuple_arg", + argstr="--tuple-arg", + type=tuple[int, str], + position=5, + ), + ] + outargs + [ShellDef.additional_args] assert sorted_fields(Cp.Outputs) == outargs + [ shell.out( name="return_code", @@ -351,25 +353,26 @@ def test_interface_template_with_defaults(): validator=attrs.validators.min_len(1), default="cp", type=str | ty.Sequence[str], - argpos=0, + position=0, help=shell.EXECUTABLE_HELP_STRING, ), shell.arg( - name="in_fs_objects", type=MultiInputObj[FsObject], argpos=1, sep=" " + name="in_fs_objects", type=MultiInputObj[FsObject], position=1, sep=" " ), output, - shell.arg(name="recursive", argstr="-R", type=bool, default=True, argpos=3), + shell.arg(name="recursive", argstr="-R", type=bool, default=True, position=3), shell.arg( - name="text_arg", argstr="--text-arg", type=str, argpos=4, default="foo" + name="text_arg", argstr="--text-arg", type=str, position=4, default="foo" ), - shell.arg(name="int_arg", argstr="--int-arg", type=int, argpos=5, default=99), + shell.arg(name="int_arg", argstr="--int-arg", type=int, position=5, default=99), shell.arg( name="tuple_arg", argstr="--tuple-arg", type=tuple[int, str], default=(1, "bar"), - argpos=6, + position=6, ), + ShellDef.additional_args, ] assert sorted_fields(Cp.Outputs) == [ output, @@ -419,27 +422,28 @@ def test_interface_template_with_type_overrides(): validator=attrs.validators.min_len(1), default="cp", type=str | ty.Sequence[str], - argpos=0, + position=0, help=shell.EXECUTABLE_HELP_STRING, ), shell.arg( - name="in_fs_objects", type=MultiInputObj[FsObject], argpos=1, sep=" " + name="in_fs_objects", type=MultiInputObj[FsObject], position=1, sep=" " ), output, - shell.arg(name="recursive", argstr="-R", type=bool, default=False, argpos=3), - shell.arg(name="text_arg", argstr="--text-arg", type=str, argpos=4), + shell.arg(name="recursive", argstr="-R", type=bool, default=False, position=3), + shell.arg(name="text_arg", argstr="--text-arg", type=str, position=4), shell.arg( name="int_arg", argstr="--int-arg", type=int | None, - argpos=5, + position=5, ), shell.arg( name="tuple_arg", argstr="--tuple-arg", type=tuple[int, str], - argpos=6, + position=6, ), + ShellDef.additional_args, ] assert sorted_fields(Cp.Outputs) == [ output, @@ -472,7 +476,7 @@ class Ls(ShellDef["Ls.Outputs"]): directory: Directory = shell.arg( help="the directory to list the contents of", argstr="", - argpos=-1, + position=-1, ) hidden: bool = shell.arg( help=("display hidden FS objects"), @@ -523,15 +527,17 @@ class Outputs(ShellOutputs): type=Directory, help="the directory to list the contents of", argstr="", - argpos=-1, + position=-1, ), "hidden": shell.arg( type=bool, help="display hidden FS objects", + default=False, argstr="-a", ), "long_format": { # Mix it up with a full dictionary based definition "type": bool, + "default": False, "help": ( "display properties of FS object, such as permissions, size and " "timestamps " @@ -541,6 +547,7 @@ class Outputs(ShellOutputs): "human_readable": shell.arg( type=bool, help="display file sizes in human readable form", + default=False, argstr="-h", requires=["long_format"], ), @@ -555,6 +562,7 @@ class Outputs(ShellOutputs): "date_format_str": shell.arg( type=str | None, help="format string for ", + default=None, argstr="-D", requires=["long_format"], xor=["complete_date"], @@ -579,6 +587,7 @@ class Outputs(ShellOutputs): def test_shell_fields(Ls): assert sorted([a.name for a in sorted_fields(Ls)]) == sorted( [ + "additional_args", "executable", "directory", "hidden", @@ -626,9 +635,9 @@ def test_shell_run(Ls, tmp_path): # Drop Long format flag to make output simpler ls = Ls(directory=tmp_path) - result = ls() + outputs = ls() - assert sorted(result.output.entries) == ["a", "b", "c"] + assert sorted(outputs.entries) == ["a", "b", "c"] @pytest.fixture(params=["static", "dynamic"]) @@ -647,7 +656,7 @@ class A: executable = "cp" - x: File = shell.arg(argstr="", argpos=1) + x: File = shell.arg(argstr="", position=1) class Outputs: """The outputs of the example shell interface @@ -667,7 +676,7 @@ class Outputs: type=File, help="an input file", argstr="", - argpos=1, + position=1, ), }, outputs={ @@ -697,7 +706,7 @@ class A: executable = "cp" - x: File = shell.arg(help="an input file", argstr="", argpos=1) + x: File = shell.arg(help="an input file", argstr="", position=1) class Outputs: y: File = shell.outarg( @@ -707,7 +716,11 @@ class Outputs: position=-1, ) - assert sorted([a.name for a in attrs.fields(A)]) == ["executable", "x", "y"] + assert sorted([a.name for a in attrs.fields(A) if not a.name.startswith("_")]) == [ + "executable", + "x", + "y", + ] assert sorted(a.name for a in attrs.fields(A.Outputs)) == [ "return_code", "stderr", @@ -729,7 +742,7 @@ class Outputs: default="cp", type=str | ty.Sequence[str], argstr="", - argpos=0, + position=0, help=shell.EXECUTABLE_HELP_STRING, ), shell.arg( @@ -737,9 +750,10 @@ class Outputs: type=File, help="an input file", argstr="", - argpos=1, + position=1, ), output, + ShellDef.additional_args, ] assert sorted_fields(A.Outputs) == [ output, @@ -770,7 +784,7 @@ def test_shell_output_field_name_dynamic(): type=File, help="an input file", argstr="", - argpos=1, + position=1, ), }, outputs={ @@ -794,7 +808,7 @@ def get_file_size(y: Path): def test_shell_bases_dynamic(A, tmp_path): B = shell.define( name="B", - inputs={"y": shell.arg(type=File, help="output file", argstr="", argpos=-1)}, + inputs={"y": shell.arg(type=File, help="output file", argstr="", position=-1)}, outputs={ "out_file_size": { "type": int, @@ -815,8 +829,8 @@ def test_shell_bases_dynamic(A, tmp_path): assert b.x == File(xpath) assert b.y == File(ypath) - # result = b() - # assert result.output.y == str(ypath) + # outputs = b() + # assert outputs.y == str(ypath) def test_shell_bases_static(A, tmp_path): @@ -848,8 +862,8 @@ class Outputs: # gets coerced to a text.Plain object assert b.y == text.Plain(ypath) - # result = b() - # assert result.output.y == str(ypath) + # outputs = b() + # assert outputs.y == str(ypath) def test_shell_inputs_outputs_bases_dynamic(tmp_path): @@ -861,7 +875,7 @@ def test_shell_inputs_outputs_bases_dynamic(tmp_path): type=Directory, help="input directory", argstr="", - argpos=-1, + position=-1, ) }, outputs={ @@ -892,9 +906,9 @@ def test_shell_inputs_outputs_bases_dynamic(tmp_path): assert b.hidden # File.sample(tmp_path, stem=".hidden-file") - # result = b() + # outputs = b() # assert result.runner.cmdline == f"ls -a {tmp_path}" - # assert result.output.entries == [".", "..", ".hidden-file"] + # assert outputs.entries == [".", "..", ".hidden-file"] def test_shell_inputs_outputs_bases_static(tmp_path): @@ -902,7 +916,7 @@ def test_shell_inputs_outputs_bases_static(tmp_path): class A: executable = "ls" - directory: Directory = shell.arg(help="input directory", argstr="", argpos=-1) + directory: Directory = shell.arg(help="input directory", argstr="", position=-1) class Outputs: entries: list = shell.out( @@ -925,8 +939,8 @@ class B(A): assert b.directory == Directory(tmp_path) assert b.hidden - # result = b() - # assert result.output.entries == [".", "..", ".hidden"] + # outputs = b() + # assert outputs.entries == [".", "..", ".hidden"] def test_shell_missing_executable_static(): @@ -935,7 +949,7 @@ def test_shell_missing_executable_static(): @shell.define class A: directory: Directory = shell.arg( - help="input directory", argstr="", argpos=-1 + help="input directory", argstr="", position=-1 ) class Outputs: @@ -957,7 +971,7 @@ def test_shell_missing_executable_dynamic(): type=Directory, help="input directory", argstr="", - argpos=-1, + position=-1, ), }, outputs={ @@ -976,9 +990,11 @@ def list_entries(stdout): def sorted_fields(interface): fields = list_fields(interface) - length = len(fields) + length = len(fields) - 1 def pos_key(out: shell.out) -> int: + if out.name == "additional_args": + return (length + 1, out.name) try: pos = out.position except AttributeError: diff --git a/pydra/engine/helpers.py b/pydra/engine/helpers.py index 4acfcc342..df27ebeec 100644 --- a/pydra/engine/helpers.py +++ b/pydra/engine/helpers.py @@ -164,7 +164,7 @@ def save( if result: if task_path.name.startswith("Workflow") and result.outputs is not None: # copy files to the workflow directory - result = copyfile_workflow(wf_path=task_path, result=result) + result.outputs = copyfile_workflow(wf_path=task_path, result=result) with (task_path / f"{name_prefix}_result.pklz").open("wb") as fp: cp.dump(result, fp) if task: @@ -172,7 +172,7 @@ def save( cp.dump(task, fp) -def copyfile_workflow(wf_path: os.PathLike, result): +def copyfile_workflow(wf_path: os.PathLike, result: "Result") -> "Result": """if file in the wf results, the file will be copied to the workflow directory""" from .helpers_file import copy_nested_files diff --git a/pydra/engine/specs.py b/pydra/engine/specs.py index 07e7c1a6f..0ac70438d 100644 --- a/pydra/engine/specs.py +++ b/pydra/engine/specs.py @@ -109,12 +109,6 @@ def __getitem__(self, name_or_index: str | int) -> ty.Any: f"{self} doesn't have an attribute {name_or_index}" ) from None - def __iter__(self) -> ty.Generator[ty.Any, None, None]: - """Iterate through all the values in the definition, allows for tuple unpacking""" - fields = sorted(attrs_fields(self), key=attrgetter("order")) - for field in fields: - yield getattr(self, field.name) - OutputsType = ty.TypeVar("OutputType", bound=TaskOutputs) @@ -427,7 +421,7 @@ def _check_rules(self): field: Arg errors = [] for field in list_fields(self): - value = getattr(self, field.name) + value = self[field.name] if is_lazy(value): continue @@ -437,7 +431,7 @@ def _check_rules(self): # Collect alternative fields associated with this field. if field.xor: - mutually_exclusive = {name: getattr(self, name) for name in field.xor} + mutually_exclusive = {name: self[name] for name in field.xor} are_set = [ f"{n}={v!r}" for n, v in mutually_exclusive.items() if v is not None ] @@ -446,7 +440,7 @@ def _check_rules(self): f"Mutually exclusive fields {field.xor} are set together: " + ", ".join(are_set) ) - elif not are_set: + elif field.mandatory and not are_set: errors.append( f"At least one of the mutually exclusive fields {field.xor} " f"should be set" @@ -588,6 +582,12 @@ class RuntimeSpec: class PythonOutputs(TaskOutputs): + def __iter__(self) -> ty.Generator[ty.Any, None, None]: + """Iterate through all the values in the definition, allows for tuple unpacking""" + fields = sorted(attrs_fields(self), key=attrgetter("order")) + for field in fields: + yield getattr(self, field.name) + @classmethod def _from_task(cls, task: "Task[PythonDef]") -> Self: """Collect the outputs of a task from a combination of the provided inputs, @@ -725,9 +725,11 @@ def construct(self) -> "Workflow": class ShellOutputs(TaskOutputs): """Output definition of a generic shell process.""" - return_code: int = shell.out(help=RETURN_CODE_HELP) - stdout: str = shell.out(help=STDOUT_HELP) - stderr: str = shell.out(help=STDERR_HELP) + BASE_NAMES = ["return_code", "stdout", "stderr"] + + return_code: int = shell.out(name="return_code", type=int, help=RETURN_CODE_HELP) + stdout: str = shell.out(name="stdout", type=str, help=STDOUT_HELP) + stderr: str = shell.out(name="stderr", type=str, help=STDERR_HELP) @classmethod def _from_task(cls, task: "Task[ShellDef]") -> Self: @@ -882,8 +884,12 @@ def _resolve_value( class ShellDef(TaskDef[ShellOutputsType]): - arguments: ty.List[str] = shell.arg( + BASE_NAMES = ["additional_args"] + + additional_args: list[str] = shell.arg( + name="additional_args", default=attrs.Factory(list), + type=list[str], sep=" ", help="Additional free-form arguments to append to the end of the command.", ) @@ -937,7 +943,7 @@ def _command_args( continue if name == "executable": pos_args.append(self._command_shelltask_executable(field, value)) - elif name == "arguments": + elif name == "additional_args": continue elif name == "args": pos_val = self._command_shelltask_args(field, value) @@ -967,7 +973,7 @@ def _command_args( cmd_args = position_sort(pos_args) # pos_args values are each a list of arguments, so concatenate lists after sorting command_args = sum(cmd_args, []) - command_args += self.arguments + command_args += inputs["additional_args"] return command_args def _command_shelltask_executable( diff --git a/pydra/engine/tests/test_dockertask.py b/pydra/engine/tests/test_dockertask.py index 94cb71b49..a80089f5a 100644 --- a/pydra/engine/tests/test_dockertask.py +++ b/pydra/engine/tests/test_dockertask.py @@ -140,7 +140,7 @@ def test_docker_inputspec_1(tmp_path): shell.arg( name="file", type=File, - argpos=1, + position=1, argstr="", help="input file", ) @@ -173,7 +173,7 @@ def test_docker_inputspec_1a(tmp_path): name="file", type=File, default=filename, - argpos=1, + position=1, argstr="", help="input file", ) @@ -206,7 +206,7 @@ def test_docker_inputspec_2(plugin, tmp_path): shell.arg( name="file1", type=File, - argpos=1, + position=1, argstr="", help="input file 1", ), @@ -214,7 +214,7 @@ def test_docker_inputspec_2(plugin, tmp_path): name="file2", type=File, default=filename_2, - argpos=2, + position=2, argstr="", help="input file 2", ), @@ -250,14 +250,14 @@ def test_docker_inputspec_2a_except(plugin, tmp_path): name="file1", type=File, default=filename_1, - argpos=1, + position=1, argstr="", help="input file 1", ), shell.arg( name="file2", type=File, - argpos=2, + position=2, argstr="", help="input file 2", ), @@ -295,14 +295,14 @@ def test_docker_inputspec_2a(plugin, tmp_path): name="file1", type=File, default=filename_1, - argpos=1, + position=1, argstr="", help="input file 1", ), shell.arg( name="file2", type=File, - argpos=2, + position=2, argstr="", help="input file 2", ), @@ -332,7 +332,7 @@ def test_docker_inputspec_3(plugin, tmp_path): shell.arg( name="file", type=File, - argpos=1, + position=1, argstr="", help="input file", container_path=True, @@ -368,7 +368,7 @@ def test_docker_cmd_inputspec_copyfile_1(plugin, tmp_path): shell.arg( name="orig_file", type=File, - argpos=1, + position=1, argstr="", help="orig file", copyfile="copy", @@ -418,7 +418,7 @@ def test_docker_inputspec_state_1(plugin, tmp_path): shell.arg( name="file", type=File, - argpos=1, + position=1, argstr="", help="input file", ) @@ -454,7 +454,7 @@ def test_docker_inputspec_state_1b(plugin, tmp_path): shell.arg( name="file", type=File, - argpos=1, + position=1, argstr="", help="input file", ) @@ -483,7 +483,7 @@ def test_docker_wf_inputspec_1(plugin, tmp_path): shell.arg( name="file", type=File, - argpos=1, + position=1, argstr="", help="input file", ) @@ -525,7 +525,7 @@ def test_docker_wf_state_inputspec_1(plugin, tmp_path): shell.arg( name="file", type=File, - argpos=1, + position=1, argstr="", help="input file", ) @@ -569,7 +569,7 @@ def test_docker_wf_ndst_inputspec_1(plugin, tmp_path): shell.arg( name="file", type=File, - argpos=1, + position=1, argstr="", help="input file", ) diff --git a/pydra/engine/tests/test_environments.py b/pydra/engine/tests/test_environments.py index 2b2d036c6..6114bf6c9 100644 --- a/pydra/engine/tests/test_environments.py +++ b/pydra/engine/tests/test_environments.py @@ -175,7 +175,7 @@ def create_shelly_inputfile(tempdir, filename, name, executable): shell.arg( name="file", type=File, - argpos=1, + position=1, help="files", argstr="", ) @@ -350,7 +350,7 @@ def create_shelly_outputfile(tempdir, filename, name, executable="cp"): shell.arg( name="file_orig", type=File, - argpos=2, + position=2, help="new file", argstr="", ), diff --git a/pydra/engine/tests/test_nipype1_convert.py b/pydra/engine/tests/test_nipype1_convert.py index 60739bd6e..07af76e50 100644 --- a/pydra/engine/tests/test_nipype1_convert.py +++ b/pydra/engine/tests/test_nipype1_convert.py @@ -16,7 +16,7 @@ def find_txt(output_dir: Path) -> File: interf_outputs = [shell.out(name="test_out", type=File, callable=find_txt)] -Interf_1 = shell.define(inputs=interf_inputs, outputs=interf_outputs) +Interf_1 = shell.define("testing", inputs=interf_inputs, outputs=interf_outputs) Interf_2 = shell.define("testing command", inputs=interf_inputs, outputs=interf_outputs) diff --git a/pydra/engine/tests/test_shelltask.py b/pydra/engine/tests/test_shelltask.py index 2c458a494..117489b3f 100644 --- a/pydra/engine/tests/test_shelltask.py +++ b/pydra/engine/tests/test_shelltask.py @@ -1601,7 +1601,7 @@ def test_shell_cmd_inputspec_11(tmp_path): sub(wf) result = wf.result() - for out_file in result.output.out: + for out_file in outputs.out: assert out_file.fspath.name == "test1" or out_file.fspath.name == "test2" @@ -4383,8 +4383,8 @@ def test_shell_cmd_optional_output_file1(tmp_path): ) file1 = tmp_path / "file1.txt" file1.write_text("foo") - result = my_cp(input=file1, unused=False) - assert result.output.output.fspath.read_text() == "foo" + outputs = my_cp(input=file1, unused=False) + assert outputs.output.fspath.read_text() == "foo" def test_shell_cmd_optional_output_file2(tmp_path): @@ -4421,8 +4421,8 @@ def test_shell_cmd_optional_output_file2(tmp_path): ) file1 = tmp_path / "file1.txt" file1.write_text("foo") - result = my_cp(input=file1, output=True) - assert result.output.output.fspath.read_text() == "foo" + outputs = my_cp(input=file1, output=True) + assert outputs.output.fspath.read_text() == "foo" file2 = tmp_path / "file2.txt" file2.write_text("bar") diff --git a/pydra/engine/tests/test_specs.py b/pydra/engine/tests/test_specs.py index 544b7570d..757c94d72 100644 --- a/pydra/engine/tests/test_specs.py +++ b/pydra/engine/tests/test_specs.py @@ -401,5 +401,5 @@ def identity(x: int) -> int: outer.add(inner.split(x=outer.lzin.x)) outer.set_output(("out", outer.inner.lzout.out)) - result = outer(x=[1, 2, 3]) - assert result.output.out == StateArray([1, 2, 3]) + outputs = outer(x=[1, 2, 3]) + assert outputs.out == StateArray([1, 2, 3]) diff --git a/pydra/engine/tests/test_submitter.py b/pydra/engine/tests/test_submitter.py index ef0c89800..9150ad008 100644 --- a/pydra/engine/tests/test_submitter.py +++ b/pydra/engine/tests/test_submitter.py @@ -704,16 +704,16 @@ def test_byo_worker(): with Submitter(worker=BYOAddVarWorker, add_var=10) as sub: assert sub.plugin == "byo_add_env_var" - result = task1(submitter=sub) + result = sub(task1) - assert result.output.out == 11 + assert outputs.out == 11 task2 = add_env_var_task(x=2) with Submitter(worker="serial") as sub: - result = task2(submitter=sub) + result = sub(task2) - assert result.output.out == 2 + assert outputs.out == 2 def test_bad_builtin_worker(): diff --git a/pydra/engine/tests/test_task.py b/pydra/engine/tests/test_task.py index 071cc9faa..339a7bc1e 100644 --- a/pydra/engine/tests/test_task.py +++ b/pydra/engine/tests/test_task.py @@ -88,10 +88,10 @@ def testfunc( # assert funky.inputs.hash == '17772c3aec9540a8dd3e187eecd2301a09c9a25c6e371ddd86e31e3a1ecfeefa' assert funky.__class__.__name__ + "_" + funky.inputs.hash == funky.checksum - result = funky() + outputs = funky() assert hasattr(result, "output") - assert hasattr(result.output, "out_out") - assert result.output.out_out == 1.1 + assert hasattr(outputs, "out_out") + assert outputs.out_out == 1.1 assert os.path.exists(funky.cache_dir / funky.checksum / "_result.pklz") funky.result() # should not recompute @@ -100,7 +100,7 @@ def testfunc( assert funky.result() is None funky() result = funky.result() - assert result.output.out_out == 2.1 + assert outputs.out_out == 2.1 help = funky.help(returnhelp=True) assert help == [ @@ -123,16 +123,16 @@ def testfunc(a: int, b: int): return dict(sum=a + b, diff=a - b) task = testfunc(a=2, b=3) - result = task() + outputs = task() # Part of the annotation and returned, should be exposed to output. - assert result.output.sum == 5 + assert outputs.sum == 5 # Part of the annotation but not returned, should be coalesced to None - assert result.output.mul is None + assert outputs.mul is None # Not part of the annotation, should be discarded. - assert not hasattr(result.output, "diff") + assert not hasattr(outputs, "diff") def test_annotated_func_multreturn(): @@ -154,13 +154,13 @@ def testfunc( assert set(funky.output_names) == {"fractional", "integer"} assert funky.__class__.__name__ + "_" + funky.inputs.hash == funky.checksum - result = funky() + outputs = funky() assert os.path.exists(funky.cache_dir / funky.checksum / "_result.pklz") assert hasattr(result, "output") - assert hasattr(result.output, "fractional") - assert result.output.fractional == 0.5 - assert hasattr(result.output, "integer") - assert result.output.integer == 3 + assert hasattr(outputs, "fractional") + assert outputs.fractional == 0.5 + assert hasattr(outputs, "integer") + assert outputs.integer == 3 help = funky.help(returnhelp=True) assert help == [ @@ -453,10 +453,10 @@ def testfunc(a, b) -> int: assert set(funky.output_names) == {"out"} assert funky.__class__.__name__ + "_" + funky.inputs.hash == funky.checksum - result = funky() + outputs = funky() assert hasattr(result, "output") - assert hasattr(result.output, "out") - assert result.output.out == 30 + assert hasattr(outputs, "out") + assert outputs.out == 30 assert os.path.exists(funky.cache_dir / funky.checksum / "_result.pklz") @@ -465,7 +465,7 @@ def testfunc(a, b) -> int: assert funky.result() is None funky() result = funky.result() - assert result.output.out == 31 + assert outputs.out == 31 help = funky.help(returnhelp=True) assert help == [ @@ -494,10 +494,10 @@ def testfunc(a, b) -> (int, int): assert set(funky.output_names) == {"out1", "out2"} assert funky.__class__.__name__ + "_" + funky.inputs.hash == funky.checksum - result = funky() + outputs = funky() assert hasattr(result, "output") - assert hasattr(result.output, "out1") - assert result.output.out1 == 11 + assert hasattr(outputs, "out1") + assert outputs.out1 == 11 assert os.path.exists(funky.cache_dir / funky.checksum / "_result.pklz") @@ -506,7 +506,7 @@ def testfunc(a, b) -> (int, int): assert funky.result() is None funky() result = funky.result() - assert result.output.out1 == 12 + assert outputs.out1 == 12 help = funky.help(returnhelp=True) assert help == [ @@ -533,8 +533,8 @@ def no_annots(c, d): result = natask._run() assert hasattr(result, "output") - assert hasattr(result.output, "out") - assert result.output.out == 20.2 + assert hasattr(outputs, "out") + assert outputs.out == 20.2 def test_notannotated_func_returnlist(): @@ -544,8 +544,8 @@ def no_annots(c, d): natask = no_annots(c=17, d=3.2) result = natask._run() - assert hasattr(result.output, "out") - assert result.output.out == [17, 3.2] + assert hasattr(outputs, "out") + assert outputs.out == [17, 3.2] def test_halfannotated_func_multrun_returnlist(): @@ -556,10 +556,10 @@ def no_annots(c, d) -> (list, float): natask = no_annots(c=17, d=3.2) result = natask._run() - assert hasattr(result.output, "out1") - assert hasattr(result.output, "out2") - assert result.output.out1 == [17, 3.2] - assert result.output.out2 == 20.2 + assert hasattr(outputs, "out1") + assert hasattr(outputs, "out2") + assert outputs.out1 == [17, 3.2] + assert outputs.out2 == 20.2 def test_notannotated_func_multreturn(): @@ -578,8 +578,8 @@ def no_annots(c, d): result = natask._run() assert hasattr(result, "output") - assert hasattr(result.output, "out") - assert result.output.out == (20.2, 13.8) + assert hasattr(outputs, "out") + assert outputs.out == (20.2, 13.8) def test_input_spec_func_1(): @@ -1427,10 +1427,10 @@ def test_taskhooks_3(tmpdir, capsys): foo = funaddtwo(name="foo", a=1, cache_dir=tmpdir) def myhook_postrun_task(task, result, *args): - print(f"postrun task hook, the result is {result.output.out}") + print(f"postrun task hook, the result is {outputs.out}") def myhook_postrun(task, result, *args): - print(f"postrun hook, the result is {result.output.out}") + print(f"postrun hook, the result is {outputs.out}") foo.hooks.post_run = myhook_postrun foo.hooks.post_run_task = myhook_postrun_task @@ -1572,8 +1572,8 @@ def test_object_input(): def testfunc(a: A): return a.x - result = testfunc(a=A(x=7))() - assert result.output.out == 7 + outputs = testfunc(a=A(x=7))() + assert outputs.out == 7 def test_argstr_formatting(): diff --git a/pydra/engine/tests/test_workflow.py b/pydra/engine/tests/test_workflow.py index 8c226d0f9..09ed541c4 100644 --- a/pydra/engine/tests/test_workflow.py +++ b/pydra/engine/tests/test_workflow.py @@ -112,9 +112,9 @@ def test_wf_dict_input_and_output_spec(): exc_info, "Could not coerce object, 'bad-value', to any of the union types" ) - result = wf() - assert result.output.a == "any-string" - assert result.output.b == {"foo": 1, "bar": False} + outputs = wf() + assert outputs.a == "any-string" + assert outputs.b == {"foo": 1, "bar": False} def test_wf_name_conflict1(): @@ -3931,11 +3931,11 @@ def test_workflow_combine1(tmpdir): } ) wf1.cache_dir = tmpdir - result = wf1() + outputs = wf1() - assert result.output.out_pow == [1, 1, 4, 8] - assert result.output.out_iden1 == [[1, 4], [1, 8]] - assert result.output.out_iden2 == [[1, 4], [1, 8]] + assert outputs.out_pow == [1, 1, 4, 8] + assert outputs.out_iden1 == [[1, 4], [1, 8]] + assert outputs.out_iden2 == [[1, 4], [1, 8]] def test_workflow_combine2(tmpdir): @@ -3946,10 +3946,10 @@ def test_workflow_combine2(tmpdir): wf1.add(identity(name="identity", x=wf1.power.lzout.out).combine("power.b")) wf1.set_output({"out_pow": wf1.power.lzout.out, "out_iden": wf1.identity.lzout.out}) wf1.cache_dir = tmpdir - result = wf1() + outputs = wf1() - assert result.output.out_pow == [[1, 4], [1, 8]] - assert result.output.out_iden == [[1, 4], [1, 8]] + assert outputs.out_pow == [[1, 4], [1, 8]] + assert outputs.out_iden == [[1, 4], [1, 8]] # testing lzout.all to collect all of the results and let PythonTask deal with it diff --git a/pydra/utils/tests/test_typing.py b/pydra/utils/tests/test_typing.py index ccca2e70c..c4c0dd120 100644 --- a/pydra/utils/tests/test_typing.py +++ b/pydra/utils/tests/test_typing.py @@ -750,9 +750,9 @@ def test_typing_implicit_cast_from_super(tmp_path, generic_task, specific_task): in_file = MyFormatX.sample() - result = wf(in_file=in_file, plugin="serial") + outputs = wf(in_file=in_file, plugin="serial") - out_file: MyFormatX = result.output.out_file + out_file: MyFormatX = outputs.out_file assert type(out_file) is MyFormatX assert out_file.parent != in_file.parent assert type(out_file.header) is MyHeader @@ -818,9 +818,9 @@ def test_typing_cast(tmp_path, specific_task, other_specific_task): in_file = MyFormatX.sample() - result = wf(in_file=in_file, plugin="serial") + outputs = wf(in_file=in_file, plugin="serial") - out_file: MyFormatX = result.output.out_file + out_file: MyFormatX = outputs.out_file assert type(out_file) is MyFormatX assert out_file.parent != in_file.parent assert type(out_file.header) is MyHeader