From 7f14e98c21d172c6d2040abbcc17832b36e30370 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Wed, 30 Aug 2023 09:39:39 +1000 Subject: [PATCH 01/20] Omit template file names for output_file options that are set to False --- pydra/engine/helpers_file.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pydra/engine/helpers_file.py b/pydra/engine/helpers_file.py index 9360774022..6e28e7bba9 100644 --- a/pydra/engine/helpers_file.py +++ b/pydra/engine/helpers_file.py @@ -120,6 +120,7 @@ def template_update(inputs, output_dir, state_ind=None, map_copyfiles=None): field for field in attr_fields(inputs) if field.metadata.get("output_file_template") + and getattr(inputs, field.name) is not False and all( getattr(inputs, required_field) is not attr.NOTHING for required_field in field.metadata.get("requires", ()) From fb2b997922918aa6886114a1b203b3bd975fdbe5 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Thu, 31 Aug 2023 12:38:20 +1000 Subject: [PATCH 02/20] added test for output_file_template with booleans --- pydra/engine/tests/test_helpers_file.py | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/pydra/engine/tests/test_helpers_file.py b/pydra/engine/tests/test_helpers_file.py index 4614d0e1e7..d2b85558c1 100644 --- a/pydra/engine/tests/test_helpers_file.py +++ b/pydra/engine/tests/test_helpers_file.py @@ -1,8 +1,11 @@ import typing as ty import sys from pathlib import Path +import attr import pytest from fileformats.generic import File +from ..specs import SpecInfo, ShellSpec +from ..task import ShellCommandTask from ..helpers_file import ( ensure_list, MountIndentifier, @@ -343,3 +346,55 @@ def test_cifs_check(): with MountIndentifier.patch_table(fake_table): for target, expected in cifs_targets: assert MountIndentifier.on_cifs(target) is expected + + +def test_output_template(tmp_path): + filename = str(tmp_path / "file.txt") + with open(filename, "w") as f: + f.write("hello from pydra") + in_file = File(filename) + + my_input_spec = SpecInfo( + name="Input", + fields=[ + ( + "in_file", + attr.ib( + type=File, + metadata={ + "mandatory": True, + "position": 1, + "argstr": "", + "help_string": "input file", + }, + ), + ), + ( + "optional", + attr.ib( + type=ty.Union[Path, bool], + default=False, + metadata={ + "position": 2, + "argstr": "--opt", + "output_file_template": "{in_file}.out", + "help_string": "optional file output", + }, + ), + ), + ], + bases=(ShellSpec,), + ) + + class MyCommand(ShellCommandTask): + executable = "my" + input_spec = my_input_spec + + task = MyCommand(in_file=filename) + assert task.cmdline == f"my {filename}" + task.inputs.optional = True + assert task.cmdline == f"my {filename} --opt {task.output_dir}/file.out" + task.inputs.optional = False + assert task.cmdline == f"my {filename}" + task.inputs.optional = "custom-file-out.txt" + assert task.cmdline == f"my {filename} --opt custom-file-out.txt" From 62e394440ab9e78072d79de6715e2f702df03c1d Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:13:42 +1000 Subject: [PATCH 03/20] fixed argstr for output_file_template with union[str, bool] type --- pydra/engine/helpers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pydra/engine/helpers.py b/pydra/engine/helpers.py index 42786f17c9..4eaf125644 100644 --- a/pydra/engine/helpers.py +++ b/pydra/engine/helpers.py @@ -652,7 +652,11 @@ def argstr_formatting(argstr, inputs, value_updates=None): for fld in inp_fields: fld_name = fld[1:-1] # extracting the name form {field_name} fld_value = inputs_dict[fld_name] - if fld_value is attr.NOTHING: + fld_attr = getattr(attrs.fields(type(inputs)), fld_name) + if fld_value is attr.NOTHING or ( + fld_value is False + and TypeParser.matches_type(fld_attr.type, ty.Union[Path, bool]) + ): # if value is NOTHING, nothing should be added to the command val_dict[fld_name] = "" else: From f4b08bf9db424eb322efd509d22de202a86c726a Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 04/20] fixed up the type-checking of fields with output_file_template --- pydra/engine/helpers_file.py | 45 +++++++++++++++++------------------- pydra/engine/specs.py | 23 +++++++++++------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/pydra/engine/helpers_file.py b/pydra/engine/helpers_file.py index 6e28e7bba9..f671aa4916 100644 --- a/pydra/engine/helpers_file.py +++ b/pydra/engine/helpers_file.py @@ -151,25 +151,21 @@ def template_update_single( # if input_dict_st with state specific value is not available, # the dictionary will be created from inputs object from ..utils.typing import TypeParser # noqa - from pydra.engine.specs import LazyField - - VALID_TYPES = (str, ty.Union[str, bool], Path, ty.Union[Path, bool], LazyField) + from pydra.engine.specs import LazyField, OUTPUT_TEMPLATE_TYPES if inputs_dict_st is None: inputs_dict_st = attr.asdict(inputs, recurse=False) if spec_type == "input": inp_val_set = inputs_dict_st[field.name] - if inp_val_set is not attr.NOTHING and not TypeParser.is_instance( - inp_val_set, VALID_TYPES - ): - raise TypeError( - f"'{field.name}' field has to be a Path instance or a bool, but {inp_val_set} set" - ) if isinstance(inp_val_set, bool) and field.type in (Path, str): raise TypeError( f"type of '{field.name}' is Path, consider using Union[Path, bool]" ) + if inp_val_set is not attr.NOTHING and not isinstance(LazyField): + inp_val_set = TypeParser(ty.Union.__getitem__(OUTPUT_TEMPLATE_TYPES))( + inp_val_set + ) elif spec_type == "output": if not TypeParser.contains_type(FileSet, field.type): raise TypeError( @@ -179,22 +175,23 @@ def template_update_single( else: raise TypeError(f"spec_type can be input or output, but {spec_type} provided") # for inputs that the value is set (so the template is ignored) - if spec_type == "input" and isinstance(inputs_dict_st[field.name], (str, Path)): - return inputs_dict_st[field.name] - elif spec_type == "input" and inputs_dict_st[field.name] is False: - # if input fld is set to False, the fld shouldn't be used (setting NOTHING) - return attr.NOTHING - else: # inputs_dict[field.name] is True or spec_type is output - value = _template_formatting(field, inputs, inputs_dict_st) - # changing path so it is in the output_dir - if output_dir and value is not attr.NOTHING: - # should be converted to str, it is also used for input fields that should be str - if type(value) is list: - return [str(output_dir / Path(val).name) for val in value] - else: - return str(output_dir / Path(value).name) - else: + if spec_type == "input": + if isinstance(inp_val_set, (Path, list)): + return inp_val_set + if inp_val_set is False: + # if input fld is set to False, the fld shouldn't be used (setting NOTHING) return attr.NOTHING + # inputs_dict[field.name] is True or spec_type is output + value = _template_formatting(field, inputs, inputs_dict_st) + # changing path so it is in the output_dir + if output_dir and value is not attr.NOTHING: + # should be converted to str, it is also used for input fields that should be str + if type(value) is list: + return [str(output_dir / Path(val).name) for val in value] + else: + return str(output_dir / Path(value).name) + else: + return attr.NOTHING def _template_formatting(field, inputs, inputs_dict_st): diff --git a/pydra/engine/specs.py b/pydra/engine/specs.py index 1877e8afa8..af9241d6a7 100644 --- a/pydra/engine/specs.py +++ b/pydra/engine/specs.py @@ -46,6 +46,13 @@ class MultiOutputType: MultiOutputObj = ty.Union[list, object, MultiOutputType] MultiOutputFile = ty.Union[File, ty.List[File], MultiOutputType] +OUTPUT_TEMPLATE_TYPES = ( + Path, + ty.List[Path], + ty.Union[Path, bool], + ty.Union[ty.List[Path], bool], +) + @attr.s(auto_attribs=True, kw_only=True) class SpecInfo: @@ -343,6 +350,8 @@ def check_metadata(self): Also sets the default values when available and needed. """ + from ..utils.typing import TypeParser + supported_keys = { "allowed_values", "argstr", @@ -361,6 +370,7 @@ def check_metadata(self): "formatter", "_output_type", } + for fld in attr_fields(self, exclude_names=("_func", "_graph_checksums")): mdata = fld.metadata # checking keys from metadata @@ -377,16 +387,13 @@ def check_metadata(self): ) # assuming that fields with output_file_template shouldn't have default if mdata.get("output_file_template"): - if fld.type not in ( - Path, - ty.Union[Path, bool], - str, - ty.Union[str, bool], + if not any( + TypeParser.matches_type(fld.type, t) for t in OUTPUT_TEMPLATE_TYPES ): raise TypeError( - f"Type of '{fld.name}' should be either pathlib.Path or " - f"typing.Union[pathlib.Path, bool] (not {fld.type}) because " - f"it has a value for output_file_template ({mdata['output_file_template']!r})" + f"Type of '{fld.name}' should be one of {OUTPUT_TEMPLATE_TYPES} " + f"(not {fld.type}) because it has a value for output_file_template " + f"({mdata['output_file_template']!r})" ) if fld.default not in [attr.NOTHING, True, False]: raise AttributeError( From b35bc73119e847d34fb0be718a5b842bb72e6cc8 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 05/20] added support for tuple/list output_file_templates for options with multiple args (e.g. mrconvert --export_grad_fsl bvec.bvec bval.bval) --- pydra/engine/helpers_file.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pydra/engine/helpers_file.py b/pydra/engine/helpers_file.py index f671aa4916..231b4e3336 100644 --- a/pydra/engine/helpers_file.py +++ b/pydra/engine/helpers_file.py @@ -162,7 +162,7 @@ def template_update_single( raise TypeError( f"type of '{field.name}' is Path, consider using Union[Path, bool]" ) - if inp_val_set is not attr.NOTHING and not isinstance(LazyField): + if inp_val_set is not attr.NOTHING and not isinstance(inp_val_set, LazyField): inp_val_set = TypeParser(ty.Union.__getitem__(OUTPUT_TEMPLATE_TYPES))( inp_val_set ) @@ -202,16 +202,27 @@ def _template_formatting(field, inputs, inputs_dict_st): Allowing for multiple input values used in the template as longs as there is no more than one file (i.e. File, PathLike or string with extensions) """ - from .specs import MultiInputObj, MultiOutputFile - # if a template is a function it has to be run first with the inputs as the only arg template = field.metadata["output_file_template"] if callable(template): template = template(inputs) # as default, we assume that keep_extension is True - keep_extension = field.metadata.get("keep_extension", True) + if isinstance(template, (tuple, list)): + formatted = [ + _string_template_formatting(field, t, inputs, inputs_dict_st) + for t in template + ] + else: + assert isinstance(template, str) + formatted = _string_template_formatting(field, template, inputs, inputs_dict_st) + return formatted + +def _string_template_formatting(field, template, inputs, inputs_dict_st): + from .specs import MultiInputObj, MultiOutputFile + + keep_extension = field.metadata.get("keep_extension", True) inp_fields = re.findall(r"{\w+}", template) inp_fields_fl = re.findall(r"{\w+:[0-9.]+f}", template) inp_fields += [re.sub(":[0-9.]+f", "", el) for el in inp_fields_fl] From 7e81e3d719261f37ad66388d9a845454874b204d Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 06/20] added test to hit multiple files in output_file_template --- pydra/engine/tests/test_helpers_file.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pydra/engine/tests/test_helpers_file.py b/pydra/engine/tests/test_helpers_file.py index d2b85558c1..4c084bd2be 100644 --- a/pydra/engine/tests/test_helpers_file.py +++ b/pydra/engine/tests/test_helpers_file.py @@ -2,6 +2,7 @@ import sys from pathlib import Path import attr +from unittest.mock import Mock import pytest from fileformats.generic import File from ..specs import SpecInfo, ShellSpec @@ -10,6 +11,7 @@ ensure_list, MountIndentifier, copy_nested_files, + template_update_single, ) @@ -398,3 +400,20 @@ class MyCommand(ShellCommandTask): assert task.cmdline == f"my {filename}" task.inputs.optional = "custom-file-out.txt" assert task.cmdline == f"my {filename} --opt custom-file-out.txt" + + +def test_template_formatting(tmp_path): + field = Mock() + field.name = "grad" + field.argstr = "--grad" + field.metadata = {"output_file_template": ("{in_file}.bvec", "{in_file}.bval")} + inputs = Mock() + inputs_dict = {"in_file": "/a/b/c/file.txt", "grad": True} + + assert template_update_single( + field, + inputs, + inputs_dict_st=inputs_dict, + output_dir=tmp_path, + spec_type="input", + ) == [f"{tmp_path}/file.bvec", f"{tmp_path}/file.bval"] From b0c5c577efc9d71a3de150e74b73dbb997292ede Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 07/20] fixed up windows compatible paths --- pydra/engine/tests/test_helpers_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydra/engine/tests/test_helpers_file.py b/pydra/engine/tests/test_helpers_file.py index 4c084bd2be..f20ec10124 100644 --- a/pydra/engine/tests/test_helpers_file.py +++ b/pydra/engine/tests/test_helpers_file.py @@ -416,4 +416,4 @@ def test_template_formatting(tmp_path): inputs_dict_st=inputs_dict, output_dir=tmp_path, spec_type="input", - ) == [f"{tmp_path}/file.bvec", f"{tmp_path}/file.bval"] + ) == [str(tmp_path / "file.bvec"), str(tmp_path / "file.bval")] From 16504d8fcd489d59f2dfee3e4d9e9e2f806222a7 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 08/20] added fileformats "fields" to coercible defaults --- pydra/utils/typing.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index ddd780ed26..f99a923cdc 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -11,6 +11,7 @@ MultiInputObj, MultiOutputObj, ) +from fileformats import field try: from typing import get_origin, get_args @@ -62,15 +63,28 @@ class TypeParser(ty.Generic[T]): not_coercible: ty.List[ty.Tuple[TypeOrAny, TypeOrAny]] COERCIBLE_DEFAULT: ty.Tuple[ty.Tuple[type, type], ...] = ( - (ty.Sequence, ty.Sequence), # type: ignore - (ty.Mapping, ty.Mapping), - (Path, os.PathLike), - (str, os.PathLike), - (os.PathLike, Path), - (os.PathLike, str), - (ty.Any, MultiInputObj), - (int, float), + ( + (ty.Sequence, ty.Sequence), # type: ignore + (ty.Mapping, ty.Mapping), + (Path, os.PathLike), + (str, os.PathLike), + (os.PathLike, Path), + (os.PathLike, str), + (ty.Any, MultiInputObj), + (int, float), + (field.Integer, float), + (int, field.Decimal), + ) + + tuple( + (f, f.primitive) + for f in (field.Integer, field.Decimal, field.Boolean, field.Text) + ) + + tuple( + (f.primitive, f) + for f in (field.Integer, field.Decimal, field.Boolean, field.Text) + ) ) + if HAVE_NUMPY: COERCIBLE_DEFAULT += ( (numpy.integer, int), From 32e10d3744dacd7f4404f18207ff0cc107ec9b1c Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 09/20] tidied up field coercing --- pydra/utils/typing.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index f99a923cdc..c69a15d21f 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -75,14 +75,8 @@ class TypeParser(ty.Generic[T]): (field.Integer, float), (int, field.Decimal), ) - + tuple( - (f, f.primitive) - for f in (field.Integer, field.Decimal, field.Boolean, field.Text) - ) - + tuple( - (f.primitive, f) - for f in (field.Integer, field.Decimal, field.Boolean, field.Text) - ) + + tuple((f, f.primitive) for f in field.Singular.subclasses() if f.primitive) + + tuple((f.primitive, f) for f in field.Singular.subclasses() if f.primitive) ) if HAVE_NUMPY: From 29e3af41fa33e01502e55c6baae66d3e56fb2d89 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 10/20] fixed up handling of type types (e.g. ty.Type[*]) --- pydra/utils/tests/test_typing.py | 24 ++++++++++++++++++++++++ pydra/utils/typing.py | 26 +++++++++++++++++++++----- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/pydra/utils/tests/test_typing.py b/pydra/utils/tests/test_typing.py index 61f1ebd119..db616fc54e 100644 --- a/pydra/utils/tests/test_typing.py +++ b/pydra/utils/tests/test_typing.py @@ -597,3 +597,27 @@ def test_typing_cast(tmp_path, generic_task, specific_task): assert out_file.parent != in_file.parent assert type(out_file.header) is MyHeader assert out_file.header.parent != in_file.header.parent + + +def test_type_is_subclass1(): + assert TypeParser.is_subclass(ty.Type[File], type) + + +def test_type_is_subclass2(): + assert not TypeParser.is_subclass(ty.Type[File], ty.Type[Json]) + + +def test_type_is_subclass3(): + assert TypeParser.is_subclass(ty.Type[Json], ty.Type[File]) + + +def test_type_is_instance1(): + assert TypeParser.is_instance(File, ty.Type[File]) + + +def test_type_is_instance2(): + assert not TypeParser.is_instance(File, ty.Type[Json]) + + +def test_type_is_instance3(): + assert TypeParser.is_instance(Json, ty.Type[File]) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index c69a15d21f..0958d05d62 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -547,9 +547,11 @@ def matches_type( return False return True - @staticmethod + @classmethod def is_instance( - obj: object, candidates: ty.Union[ty.Type[ty.Any], ty.Iterable[ty.Type[ty.Any]]] + cls, + obj: object, + candidates: ty.Union[ty.Type[ty.Any], ty.Iterable[ty.Type[ty.Any]]], ) -> bool: """Checks whether the object is an instance of cls or that cls is typing.Any, extending the built-in isinstance to check nested type args @@ -566,9 +568,14 @@ def is_instance( for candidate in candidates: if candidate is ty.Any: return True + # Handle ty.Type[*] candidates + if ty.get_origin(candidate) is type: + return inspect.isclass(obj) and cls.is_subclass( + obj, ty.get_args(candidate)[0] + ) if NO_GENERIC_ISSUBCLASS: - if candidate is type and inspect.isclass(obj): - return True + if inspect.isclass(obj): + return candidate is type if issubtype(type(obj), candidate) or ( type(obj) is dict and candidate is ty.Mapping ): @@ -597,10 +604,19 @@ def is_subclass( any_ok : bool whether klass=typing.Any should return True or False """ - if not isinstance(candidates, ty.Iterable): + if not isinstance(candidates, ty.Sequence): candidates = [candidates] for candidate in candidates: + # Handle ty.Type[*] types in klass and candidates + if ty.get_origin(klass) is type and ( + candidate is type or ty.get_origin(candidate) is type + ): + if candidate is type: + return True + return cls.is_subclass(ty.get_args(klass)[0], ty.get_args(candidate)[0]) + elif ty.get_origin(klass) is type or ty.get_origin(candidate) is type: + return False if NO_GENERIC_ISSUBCLASS: if klass is type and candidate is not type: return False From 9155bd00412ad3284475fd806cd0dabcc952ef82 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 11/20] added another test --- pydra/utils/tests/test_typing.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pydra/utils/tests/test_typing.py b/pydra/utils/tests/test_typing.py index db616fc54e..02de178edb 100644 --- a/pydra/utils/tests/test_typing.py +++ b/pydra/utils/tests/test_typing.py @@ -621,3 +621,7 @@ def test_type_is_instance2(): def test_type_is_instance3(): assert TypeParser.is_instance(Json, ty.Type[File]) + + +def test_type_is_instance4(): + assert TypeParser.is_instance(Json, type) From a53ec8dc673ee442c736f23cb0eeb997e1ec5e39 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 12/20] error message touch-up --- pydra/utils/typing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index 0958d05d62..e429d6a817 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -189,18 +189,18 @@ def expand_and_coerce(obj, pattern: ty.Union[type, tuple]): obj_args = list(obj) except TypeError as e: msg = ( - f" (part of coercion from {object_} to {self.pattern}" + f" (part of coercion from {object_!r} to {self.pattern}" if obj is not object_ else "" ) raise TypeError( - f"Could not coerce to {type_} as {obj} is not iterable{msg}" + f"Could not coerce to {type_} as {obj!r} is not iterable{msg}" ) from e if issubclass(origin, tuple): return coerce_tuple(type_, obj_args, pattern_args) if issubclass(origin, ty.Iterable): return coerce_sequence(type_, obj_args, pattern_args) - assert False, f"Coercion from {obj} to {pattern} is not handled" + assert False, f"Coercion from {obj!r} to {pattern} is not handled" def coerce_basic(obj, pattern): """Coerce an object to a "basic types" like `int`, `float`, `bool`, `Path` From e4a2a09c6149f8ece6b67e4cf6832d481c7ef305 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 13/20] added label to type parser --- pydra/engine/helpers.py | 3 ++- pydra/engine/specs.py | 3 ++- pydra/utils/typing.py | 45 +++++++++++++++++++++++++++-------------- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/pydra/engine/helpers.py b/pydra/engine/helpers.py index 4eaf125644..c6515b819d 100644 --- a/pydra/engine/helpers.py +++ b/pydra/engine/helpers.py @@ -261,7 +261,8 @@ def make_klass(spec): type=tp, **kwargs, ) - type_checker = TypeParser[newfield.type](newfield.type) + checker_label = f"'{name}' field of {spec.name}" + type_checker = TypeParser[newfield.type](newfield.type, label=checker_label) if newfield.type in (MultiInputObj, MultiInputFile): converter = attr.converters.pipe(ensure_list, type_checker) elif newfield.type in (MultiOutputObj, MultiOutputFile): diff --git a/pydra/engine/specs.py b/pydra/engine/specs.py index af9241d6a7..54181bde21 100644 --- a/pydra/engine/specs.py +++ b/pydra/engine/specs.py @@ -450,7 +450,8 @@ def collect_additional_outputs(self, inputs, output_dir, outputs): input_value = getattr(inputs, fld.name, attr.NOTHING) if input_value is not attr.NOTHING: if TypeParser.contains_type(FileSet, fld.type): - input_value = TypeParser(fld.type).coerce(input_value) + label = f"output field '{fld.name}' of {self}" + input_value = TypeParser(fld.type, label=label).coerce(input_value) additional_out[fld.name] = input_value elif ( fld.default is None or fld.default == attr.NOTHING diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index e429d6a817..9a1ec358eb 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -56,11 +56,15 @@ class TypeParser(ty.Generic[T]): the tree of more complex nested container types. Overrides 'coercible' to enable you to carve out exceptions, such as TypeParser(list, coercible=[(ty.Iterable, list)], not_coercible=[(str, list)]) + label : str + the label to be used to identify the type parser in error messages. Especially + useful when TypeParser is used as a converter in attrs.fields """ tp: ty.Type[T] coercible: ty.List[ty.Tuple[TypeOrAny, TypeOrAny]] not_coercible: ty.List[ty.Tuple[TypeOrAny, TypeOrAny]] + label: str COERCIBLE_DEFAULT: ty.Tuple[ty.Tuple[type, type], ...] = ( ( @@ -103,6 +107,7 @@ def __init__( not_coercible: ty.Optional[ ty.Iterable[ty.Tuple[TypeOrAny, TypeOrAny]] ] = NOT_COERCIBLE_DEFAULT, + label: str = "", ): def expand_pattern(t): """Recursively expand the type arguments of the target type in nested tuples""" @@ -118,10 +123,12 @@ def expand_pattern(t): return origin if origin not in (ty.Union, type) and not issubclass(origin, ty.Iterable): raise TypeError( - f"TypeParser doesn't know how to handle args ({args}) for {origin} types" + f"TypeParser doesn't know how to handle args ({args}) for {origin} " + f"types{self.label_str}" ) return (origin, [expand_pattern(a) for a in args]) + self.label = label self.tp = tp self.coercible = ( list(coercible) if coercible is not None else [(ty.Any, ty.Any)] @@ -194,7 +201,7 @@ def expand_and_coerce(obj, pattern: ty.Union[type, tuple]): else "" ) raise TypeError( - f"Could not coerce to {type_} as {obj!r} is not iterable{msg}" + f"Could not coerce to {type_} as {obj!r} is not iterable{msg}{self.label_str}" ) from e if issubclass(origin, tuple): return coerce_tuple(type_, obj_args, pattern_args) @@ -221,7 +228,8 @@ def coerce_union(obj, pattern_args): except TypeError as e: reasons.append(e) raise TypeError( - f"Could not coerce object, {obj!r}, to any of the union types {pattern_args}:\n\n" + f"Could not coerce object, {obj!r}, to any of the union types " + f"{pattern_args}{self.label_str}:\n\n" + "\n\n".join(f"{a} -> {e}" for a, e in zip(pattern_args, reasons)) ) @@ -240,7 +248,7 @@ def coerce_mapping( else "" ) raise TypeError( - f"Could not coerce to {type_} as {obj} is not a mapping type{msg}" + f"Could not coerce to {type_} as {obj} is not a mapping type{msg}{self.label_str}" ) from e return coerce_obj( { @@ -263,7 +271,7 @@ def coerce_tuple( elif len(pattern_args) != len(obj_args): raise TypeError( f"Incorrect number of items in tuple, expected " - f"{len(pattern_args)}, got {len(obj_args)}" + f"{len(pattern_args)}, got {len(obj_args)}{self.label_str}" ) return coerce_obj( [expand_and_coerce(o, p) for o, p in zip(obj_args, pattern_args)], type_ @@ -281,7 +289,7 @@ def coerce_sequence( def coerce_type(type_: ty.Type[ty.Any], pattern_args: ty.List[ty.Type[ty.Any]]): if not any(issubclass(type_, t) for t in pattern_args): raise TypeError( - f"{type_} is not one of the specified types {pattern_args}" + f"{type_} is not one of the specified types {pattern_args}{self.label_str}" ) return type_ @@ -297,7 +305,9 @@ def coerce_obj(obj, type_): if obj is not object_ else "" ) - raise TypeError(f"Cannot coerce {obj!r} into {type_}{msg}") from e + raise TypeError( + f"Cannot coerce {obj!r} into {type_}{msg}{self.label_str}" + ) from e return expand_and_coerce(object_, self.pattern) @@ -323,7 +333,7 @@ def check_type(self, type_: ty.Type[ty.Any]): raise TypeError("Splits without any type arguments are invalid") if len(args) > 1: raise TypeError( - f"Splits with more than one type argument ({args}) are invalid" + f"Splits with more than one type argument ({args}) are invalid{self.label_str}" ) return self.check_type(args[0]) @@ -343,7 +353,7 @@ def expand_and_check(tp, pattern: ty.Union[type, tuple]): ) raise TypeError( f"{tp} doesn't match pattern {pattern}, when matching {type_} to " - f"{self.pattern}" + f"{self.pattern}{self.label_str}" ) tp_args = get_args(tp) self.check_coercible(tp_origin, pattern_origin) @@ -378,7 +388,7 @@ def check_union(tp, pattern_args): if reasons: raise TypeError( f"Cannot coerce {tp} to " - f"ty.Union[{', '.join(str(a) for a in pattern_args)}], " + f"ty.Union[{', '.join(str(a) for a in pattern_args)}]{self.label_str}, " f"because {tp_arg} cannot be coerced to any of its args:\n\n" + "\n\n".join( f"{a} -> {e}" for a, e in zip(pattern_args, reasons) @@ -414,7 +424,7 @@ def check_tuple(tp_args, pattern_args): if len(tp_args) != len(pattern_args): raise TypeError( f"Wrong number of type arguments in tuple {tp_args} compared to pattern " - f"{pattern_args} in attempting to match {type_} to {self.pattern}" + f"{pattern_args} in attempting to match {type_} to {self.pattern}{self.label_str}" ) for t, p in zip(tp_args, pattern_args): expand_and_check(t, p) @@ -426,7 +436,8 @@ def check_sequence(tp_args, pattern_args): if not tp_args: raise TypeError( "Generic ellipsis type arguments not specific enough to match " - f"{pattern_args} in attempting to match {type_} to {self.pattern}" + f"{pattern_args} in attempting to match {type_} to " + f"{self.pattern}{self.label_str}" ) for arg in tp_args: expand_and_check(arg, pattern_args[0]) @@ -476,8 +487,8 @@ def type_name(t): if not matches_criteria(self.coercible): raise TypeError( - f"Cannot coerce {repr(source)} into {target} as the coercion doesn't match " - f"any of the explicit inclusion criteria: " + f"Cannot coerce {repr(source)} into {target}{self.label_str} as the " + "coercion doesn't match any of the explicit inclusion criteria: " + ", ".join( f"{type_name(s)} -> {type_name(t)}" for s, t in self.coercible ) @@ -485,7 +496,7 @@ def type_name(t): matches_not_coercible = matches_criteria(self.not_coercible) if matches_not_coercible: raise TypeError( - f"Cannot coerce {repr(source)} into {target} as it is explicitly " + f"Cannot coerce {repr(source)} into {target}{self.label_str} as it is explicitly " "excluded by the following coercion criteria: " + ", ".join( f"{type_name(s)} -> {type_name(t)}" @@ -799,5 +810,9 @@ def strip_splits(cls, type_: ty.Type[ty.Any]) -> ty.Tuple[ty.Type, int]: depth += 1 return type_, depth + @property + def label_str(self): + return f" in {self.label} " if self.label else "" + get_origin = staticmethod(get_origin) get_args = staticmethod(get_args) From b67c7255a90fa7488feb1a5ff5e40b88a3ec0832 Mon Sep 17 00:00:00 2001 From: adsouza Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 14/20] added case for ellipsis in typing object to coerce --- pydra/utils/typing.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index 9a1ec358eb..93be199d5e 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -421,6 +421,10 @@ def check_tuple(tp_args, pattern_args): for arg in tp_args: expand_and_check(arg, pattern_args[0]) return + elif tp_args[-1] is Ellipsis: + for pattern_arg in pattern_args: + expand_and_check(tp_args[0], pattern_arg) + return if len(tp_args) != len(pattern_args): raise TypeError( f"Wrong number of type arguments in tuple {tp_args} compared to pattern " From 9fabcb32acf5b0338462ff2af83c475a2361e58f Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:14:00 +1000 Subject: [PATCH 15/20] added list of list of Paths to accepted output template types --- pydra/engine/specs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pydra/engine/specs.py b/pydra/engine/specs.py index 54181bde21..c31705be7d 100644 --- a/pydra/engine/specs.py +++ b/pydra/engine/specs.py @@ -51,6 +51,7 @@ class MultiOutputType: ty.List[Path], ty.Union[Path, bool], ty.Union[ty.List[Path], bool], + ty.List[ty.List[Path]], ) From d109e53a39a5dbd919f441ebd482cacaffcfd6fb Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:45:23 +1000 Subject: [PATCH 16/20] reverted typing args with tuple ellipsis --- pydra/utils/typing.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index 93be199d5e..7ef272069d 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -421,10 +421,10 @@ def check_tuple(tp_args, pattern_args): for arg in tp_args: expand_and_check(arg, pattern_args[0]) return - elif tp_args[-1] is Ellipsis: - for pattern_arg in pattern_args: - expand_and_check(tp_args[0], pattern_arg) - return + # elif tp_args[-1] is Ellipsis: + # for pattern_arg in pattern_args: + # expand_and_check(tp_args[0], pattern_arg) + # return if len(tp_args) != len(pattern_args): raise TypeError( f"Wrong number of type arguments in tuple {tp_args} compared to pattern " From b6e62da1e61859baf418a36246cdb1360bc64fba Mon Sep 17 00:00:00 2001 From: Tom Close Date: Fri, 1 Sep 2023 11:53:17 +1000 Subject: [PATCH 17/20] fixed up paths in test_output_template so it works on windows --- pydra/engine/tests/test_helpers_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydra/engine/tests/test_helpers_file.py b/pydra/engine/tests/test_helpers_file.py index f20ec10124..ea5dd2afdc 100644 --- a/pydra/engine/tests/test_helpers_file.py +++ b/pydra/engine/tests/test_helpers_file.py @@ -395,7 +395,7 @@ class MyCommand(ShellCommandTask): task = MyCommand(in_file=filename) assert task.cmdline == f"my {filename}" task.inputs.optional = True - assert task.cmdline == f"my {filename} --opt {task.output_dir}/file.out" + assert task.cmdline == f"my {filename} --opt {task.output_dir / 'file.out'}" task.inputs.optional = False assert task.cmdline == f"my {filename}" task.inputs.optional = "custom-file-out.txt" From a36be3ebc166bc8aee9bef0dd3418a0066940bf8 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Thu, 7 Sep 2023 10:50:40 +1000 Subject: [PATCH 18/20] Update pydra/engine/helpers_file.py Co-authored-by: Chris Markiewicz --- pydra/engine/helpers_file.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pydra/engine/helpers_file.py b/pydra/engine/helpers_file.py index 231b4e3336..f194533ac7 100644 --- a/pydra/engine/helpers_file.py +++ b/pydra/engine/helpers_file.py @@ -163,9 +163,7 @@ def template_update_single( f"type of '{field.name}' is Path, consider using Union[Path, bool]" ) if inp_val_set is not attr.NOTHING and not isinstance(inp_val_set, LazyField): - inp_val_set = TypeParser(ty.Union.__getitem__(OUTPUT_TEMPLATE_TYPES))( - inp_val_set - ) + inp_val_set = TypeParser(ty.Union[OUTPUT_TEMPLATE_TYPES])(inp_val_set) elif spec_type == "output": if not TypeParser.contains_type(FileSet, field.type): raise TypeError( From 7f3809236158b4f2b35aa13399b0c73311a4f199 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Thu, 7 Sep 2023 10:50:53 +1000 Subject: [PATCH 19/20] Update pydra/utils/typing.py Co-authored-by: Chris Markiewicz --- pydra/utils/typing.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index 7ef272069d..9a1ec358eb 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -421,10 +421,6 @@ def check_tuple(tp_args, pattern_args): for arg in tp_args: expand_and_check(arg, pattern_args[0]) return - # elif tp_args[-1] is Ellipsis: - # for pattern_arg in pattern_args: - # expand_and_check(tp_args[0], pattern_arg) - # return if len(tp_args) != len(pattern_args): raise TypeError( f"Wrong number of type arguments in tuple {tp_args} compared to pattern " From 103cefcc68097f63ff2385f509aed4410e32dc70 Mon Sep 17 00:00:00 2001 From: Tom Close Date: Thu, 7 Sep 2023 10:54:28 +1000 Subject: [PATCH 20/20] removed stray mypy ignore --- pydra/utils/typing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydra/utils/typing.py b/pydra/utils/typing.py index 9a1ec358eb..ceddc7e219 100644 --- a/pydra/utils/typing.py +++ b/pydra/utils/typing.py @@ -68,7 +68,7 @@ class TypeParser(ty.Generic[T]): COERCIBLE_DEFAULT: ty.Tuple[ty.Tuple[type, type], ...] = ( ( - (ty.Sequence, ty.Sequence), # type: ignore + (ty.Sequence, ty.Sequence), (ty.Mapping, ty.Mapping), (Path, os.PathLike), (str, os.PathLike),