Merge pull request #705 from djarecka/env

Env

Author: djarecka

.pre-commit-config.yaml

@@ -9,7 +9,7 @@ repos:
     -   id: check-yaml
     -   id: check-added-large-files
 -   repo: https://github.com/psf/black
-    rev: 23.7.0
+    rev: 23.9.1
     hooks:
     -   id: black
 -   repo: https://github.com/codespell-project/codespell

pydra/engine/environments.py

@@ -55,7 +55,7 @@ def execute(self, task, root="/mnt/pydra"):
         # Skips over any inputs in task.cache_dir
         # Needs to include `out_file`s when not relative to working dir
         # Possibly a `TargetFile` type to distinguish between `File` and `str`?
-        mounts = task.get_inputs_in_root(root=root)
+        mounts = task.get_bindings(root=root)
 
         # todo adding xargsy etc
         docker_args = ["docker", "run", "-v", self.bind(task.cache_dir, "rw")]

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):
@@ -652,7 +653,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:

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", ())
@@ -150,25 +151,19 @@ 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(inp_val_set, LazyField):
+            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(
@@ -178,22 +173,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):
@@ -204,16 +200,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]

pydra/engine/specs.py

@@ -46,6 +46,14 @@ 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],
+    ty.List[ty.List[Path]],
+)
+
 
 @attr.s(auto_attribs=True, kw_only=True)
 class SpecInfo:
@@ -343,6 +351,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 +371,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 +388,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(
@@ -443,7 +451,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

pydra/engine/task.py

@@ -38,6 +38,8 @@
       <https://colab.research.google.com/drive/1RRV1gHbGJs49qQB1q1d5tQEycVRtuhw6>`__
 
 """
+from __future__ import annotations
+
 import platform
 import re
 import attr
@@ -62,7 +64,6 @@
 )
 from .helpers import (
     ensure_list,
-    execute,
     position_sort,
     argstr_formatting,
     output_from_inputfields,
@@ -354,7 +355,7 @@ def get_bindings(self, root: str | None = None) -> dict[str, tuple[str, str]]:
         if root is None:
             return {}
         else:
-            self._check_inputs(root=root)
+            self._prepare_bindings(root=root)
             return self.bindings
 
     def command_args(self, root=None):
@@ -574,29 +575,7 @@ def cmdline(self):
     def _run_task(self, environment=None):
         if environment is None:
             environment = self.environment
-
-        if (
-            environment == "old"
-        ):  # TODO this is just temporarily for testing, remove this part
-            if isinstance(self, ContainerTask):
-                args = self.container_args + self.command_args()
-            else:
-                args = self.command_args()
-            if args:
-                # removing empty strings
-                args = [str(el) for el in args if el not in ["", " "]]
-                keys = ["return_code", "stdout", "stderr"]
-                values = execute(args, strip=self.strip)
-                self.output_ = dict(zip(keys, values))
-                if self.output_["return_code"]:
-                    msg = f"Error running '{self.name}' task with {args}:"
-                    if self.output_["stderr"]:
-                        msg += "\n\nstderr:\n" + self.output_["stderr"]
-                    if self.output_["stdout"]:
-                        msg += "\n\nstdout:\n" + self.output_["stdout"]
-                    raise RuntimeError(msg)
-        else:
-            self.output_ = environment.execute(self)
+        self.output_ = environment.execute(self)
 
     def _prepare_bindings(self, root: str):
         """Prepare input files to be passed to the task
@@ -721,7 +700,7 @@ def bind_paths(self):
         mount points: dict
             mapping from local path to tuple of container path + mode
         """
-        self._check_inputs()
+        self._prepare_bindings()
         return {**self.bindings, **{self.output_dir: (self.output_cpath, "rw")}}
 
     def binds(self, opt):
@@ -736,7 +715,7 @@ def binds(self, opt):
             bargs.extend([opt, f"{lpath}:{cpath}:{mode}"])
         return bargs
 
-    def _check_inputs(self):
+    def _prepare_bindings(self):
         fields = attr_fields(self.inputs)
         for fld in fields:
             if TypeParser.contains_type(FileSet, fld.type):

pydra/engine/tests/test_dockertask.py

@@ -17,13 +17,13 @@ def test_docker_1_nosubm():
     no submitter
     """
     cmd = "whoami"
-    docky = DockerTask(name="docky", executable=cmd, image="busybox", environment="old")
-    assert docky.inputs.image == "busybox"
-    assert docky.inputs.container == "docker"
-    assert (
-        docky.cmdline
-        == f"docker run --rm -v {docky.output_dir}:/output_pydra:rw -w /output_pydra {docky.inputs.image} {cmd}"
+    docky = ShellCommandTask(
+        name="docky", executable=cmd, environment=Docker(image="busybox")
     )
+    assert docky.environment.image == "busybox"
+    assert docky.environment.tag == "latest"
+    assert isinstance(docky.environment, Docker)
+    assert docky.cmdline == cmd
 
     res = docky()
     assert res.output.stdout == "root\n"

pydra/engine/tests/test_environments.py

@@ -46,13 +46,6 @@ def test_native_1(tmp_path):
         shelly_subm(submitter=sub, environment=Native())
     assert env_res == shelly_subm.result().output.__dict__
 
-    # TODO: should be removed at the end
-    shelly_old = ShellCommandTask(
-        name="shelly_old", executable=cmd, cache_dir=tmp_path, environment="old"
-    )
-    shelly_old()
-    assert env_res == shelly_old.result().output.__dict__
-
 
 @no_win
 @need_docker