Merge pull request #320 from djarecka/enh/validator

adding a simple validator to the attrs class

Author: djarecka

docs/requirements.txt

@@ -1,4 +1,4 @@
-attrs
+attrs >= 19.1.0
 cloudpickle
 filelock
 git+https://github.com/AleksandarPetrov/napoleon.git@0dc3f28a309ad602be5f44a9049785a1026451b3#egg=sphinxcontrib-napoleon

pydra/__init__.py

@@ -35,3 +35,13 @@ def check_latest_version():
 
     if TaskBase._etelemetry_version_data is None:
         TaskBase._etelemetry_version_data = check_latest_version()
+
+
+# attr run_validators is set to False, but could be changed using use_validator
+import attr
+
+attr.set_run_validators(False)
+
+
+def set_input_validator(flag=False):
+    attr.set_run_validators(flag)

pydra/engine/helpers.py

@@ -13,9 +13,12 @@
 import re
 from time import strftime
 from traceback import format_exception
+import typing as ty
+import inspect
+import warnings
 
 
-from .specs import Runtime, File, Directory, attr_fields, Result
+from .specs import Runtime, File, Directory, attr_fields, Result, LazyField
 from .helpers_file import hash_file, hash_dir, copyfile, is_existing_file
 
 
@@ -234,8 +237,11 @@ def make_klass(spec):
             if len(item) == 2:
                 if isinstance(item[1], attr._make._CountingAttr):
                     newfields[item[0]] = item[1]
+                    newfields[item[0]].validator(custom_validator)
                 else:
-                    newfields[item[0]] = attr.ib(type=item[1])
+                    newfields[item[0]] = attr.ib(
+                        type=item[1], validator=custom_validator
+                    )
             else:
                 if (
                     any([isinstance(ii, attr._make._CountingAttr) for ii in item])
@@ -251,17 +257,201 @@ def make_klass(spec):
                         name, tp = item[:2]
                         if isinstance(item[-1], dict) and "help_string" in item[-1]:
                             mdata = item[-1]
-                            newfields[name] = attr.ib(type=tp, metadata=mdata)
+                            newfields[name] = attr.ib(
+                                type=tp, metadata=mdata, validator=custom_validator
+                            )
                         else:
                             dflt = item[-1]
-                            newfields[name] = attr.ib(type=tp, default=dflt)
+                            newfields[name] = attr.ib(
+                                type=tp, default=dflt, validator=custom_validator
+                            )
                     elif len(item) == 4:
                         name, tp, dflt, mdata = item
-                        newfields[name] = attr.ib(type=tp, default=dflt, metadata=mdata)
+                        newfields[name] = attr.ib(
+                            type=tp,
+                            default=dflt,
+                            metadata=mdata,
+                            validator=custom_validator,
+                        )
         fields = newfields
     return attr.make_class(spec.name, fields, bases=spec.bases, kw_only=True)
 
 
+def custom_validator(instance, attribute, value):
+    """simple custom validation
+    take into account ty.Union, ty.List, ty.Dict (but only one level depth)
+    adding an additional validator, if allowe_values provided
+    """
+    validators = []
+    tp_attr = attribute.type
+    # a flag that could be changed to False, if the type is not recognized
+    check_type = True
+    if (
+        value is attr.NOTHING
+        or value is None
+        or attribute.name.startswith("_")  # e.g. _func
+        or isinstance(value, LazyField)
+        or tp_attr in [ty.Any, inspect._empty]
+    ):
+        check_type = False  # no checking of the type
+    elif isinstance(tp_attr, type) or tp_attr in [File, Directory]:
+        tp = _single_type_update(tp_attr, name=attribute.name)
+        cont_type = None
+    else:  # more complex types
+        cont_type, tp_attr_list = _check_special_type(tp_attr, name=attribute.name)
+        if cont_type is ty.Union:
+            tp, check_type = _types_updates(tp_attr_list, name=attribute.name)
+        elif cont_type is list:
+            tp, check_type = _types_updates(tp_attr_list, name=attribute.name)
+        elif cont_type is dict:
+            # assuming that it should have length of 2 for keys and values
+            if len(tp_attr_list) != 2:
+                check_type = False
+            else:
+                tp_attr_key, tp_attr_val = tp_attr_list
+            # updating types separately for keys and values
+            tp_k, check_k = _types_updates([tp_attr_key], name=attribute.name)
+            tp_v, check_v = _types_updates([tp_attr_val], name=attribute.name)
+            # assuming that I have to be able to check keys and values
+            if not (check_k and check_v):
+                check_type = False
+            else:
+                tp = {"key": tp_k, "val": tp_v}
+        else:
+            warnings.warn(
+                f"no type check for {attribute.name} field, no type check implemented for value {value} and type {tp_attr}"
+            )
+            check_type = False
+
+    if check_type:
+        validators.append(_type_validator(instance, attribute, value, tp, cont_type))
+
+    # checking additional requirements for values (e.g. allowed_values)
+    meta_attr = attribute.metadata
+    if "allowed_values" in meta_attr:
+        validators.append(_allowed_values_validator(isinstance, attribute, value))
+    return validators
+
+
+def _type_validator(instance, attribute, value, tp, cont_type):
+    """ creating a customized type validator,
+    uses validator.deep_iterable/mapping if the field is a container
+    (i.e. ty.List or ty.Dict),
+    it also tries to guess when the value is a list due to the splitter
+    and validates the elements
+    """
+    if cont_type is None or cont_type is ty.Union:
+        # if tp is not (list,), we are assuming that the value is a list
+        # due to the splitter, so checking the member types
+        if isinstance(value, list) and tp != (list,):
+            return attr.validators.deep_iterable(
+                member_validator=attr.validators.instance_of(
+                    tp + (attr._make._Nothing,)
+                )
+            )(instance, attribute, value)
+        else:
+            return attr.validators.instance_of(tp + (attr._make._Nothing,))(
+                instance, attribute, value
+            )
+    elif cont_type is list:
+        return attr.validators.deep_iterable(
+            member_validator=attr.validators.instance_of(tp + (attr._make._Nothing,))
+        )(instance, attribute, value)
+    elif cont_type is dict:
+        return attr.validators.deep_mapping(
+            key_validator=attr.validators.instance_of(tp["key"]),
+            value_validator=attr.validators.instance_of(
+                tp["val"] + (attr._make._Nothing,)
+            ),
+        )(instance, attribute, value)
+    else:
+        raise Exception(
+            f"container type of {attribute.name} should be None, list, dict or ty.Union, and not {cont_type}"
+        )
+
+
+def _types_updates(tp_list, name):
+    """updating the type's tuple with possible additional types"""
+    tp_upd_list = []
+    check = True
+    for tp_el in tp_list:
+        tp_upd = _single_type_update(tp_el, name, simplify=True)
+        if tp_upd is None:
+            check = False
+            break
+        else:
+            tp_upd_list += list(tp_upd)
+    tp_upd = tuple(set(tp_upd_list))
+    return tp_upd, check
+
+
+def _single_type_update(tp, name, simplify=False):
+    """ updating a single type with other related types - e.g. adding bytes for str
+        if simplify is True, than changing typing.List to list etc.
+        (assuming that I validate only one depth, so have to simplify at some point)
+    """
+    if isinstance(tp, type) or tp in [File, Directory]:
+        if tp is str:
+            return (str, bytes)
+        elif tp in [File, Directory, os.PathLike]:
+            return (os.PathLike, str)
+        elif tp is float:
+            return (float, int)
+        else:
+            return (tp,)
+    elif simplify is True:
+        warnings.warn(f"simplify validator for {name} field, checking only one depth")
+        cont_tp, types_list = _check_special_type(tp, name=name)
+        if cont_tp is list:
+            return (list,)
+        elif cont_tp is dict:
+            return (dict,)
+        elif cont_tp is ty.Union:
+            return types_list
+        else:
+            warnings.warn(
+                f"no type check for {name} field, type check not implemented for type of {tp}"
+            )
+            return None
+    else:
+        warnings.warn(
+            f"no type check for {name} field, type check not implemented for type - {tp}, consider using simplify=True"
+        )
+        return None
+
+
+def _check_special_type(tp, name):
+    """checking if the type is a container: ty.List, ty.Dict or ty.Union """
+    if sys.version_info.minor >= 8:
+        return ty.get_origin(tp), ty.get_args(tp)
+    else:
+        if isinstance(tp, type):  # simple type
+            return None, ()
+        else:
+            if tp._name == "List":
+                return list, tp.__args__
+            elif tp._name == "Dict":
+                return dict, tp.__args__
+            elif tp.__origin__ is ty.Union:
+                return ty.Union, tp.__args__
+            else:
+                warnings.warn(
+                    f"not type check for {name} field, type check not implemented for type {tp}"
+                )
+                return None, ()
+
+
+def _allowed_values_validator(instance, attribute, value):
+    """ checking if the values is in allowed_values"""
+    allowed = attribute.metadata["allowed_values"]
+    if value is attr.NOTHING:
+        pass
+    elif value not in allowed:
+        raise ValueError(
+            f"value of {attribute.name} has to be from {allowed}, but {value} provided"
+        )
+
+
 async def read_stream_and_display(stream, display):
     """
     Read from stream line by line until EOF, display, and capture the lines.

pydra/engine/specs.py

@@ -293,26 +293,11 @@ def check_fields_input_spec(self):
             if required_notfound:
                 raise AttributeError(f"{nm} requires {required_notfound}")
 
-            # TODO: types might be checked here
-        self._type_checking()
-
     def _file_check(self, field):
         file = Path(getattr(self, field.name))
         if not file.exists():
             raise AttributeError(f"the file from the {field.name} input does not exist")
 
-    def _type_checking(self):
-        """Use fld.type to check the types TODO.
-
-        This may be done through attr validators.
-
-        """
-        fields = attr_fields(self)
-        allowed_keys = ["min_val", "max_val", "range", "enum"]  # noqa
-        for fld in fields:
-            # TODO
-            pass
-
 
 @attr.s(auto_attribs=True, kw_only=True)
 class ShellOutSpec(BaseSpec):

pydra/engine/tests/test_shelltask.py

@@ -9,8 +9,7 @@
 from ..submitter import Submitter
 from ..core import Workflow
 from ..specs import ShellOutSpec, ShellSpec, SpecInfo, File
-from .utils import result_no_submitter, result_submitter
-
+from .utils import result_no_submitter, result_submitter, use_validator
 
 if sys.platform.startswith("win"):
     pytest.skip("SLURM not available in windows", allow_module_level=True)
@@ -251,7 +250,7 @@ def test_wf_shell_cmd_1(plugin):
 
 
 @pytest.mark.parametrize("results_function", [result_no_submitter, result_submitter])
-def test_shell_cmd_inputspec_1(plugin, results_function):
+def test_shell_cmd_inputspec_1(plugin, results_function, use_validator):
     """ a command with executable, args and one command opt,
         using a customized input_spec to add the opt to the command
         in the right place that is specified in metadata["cmd_pos"]
@@ -290,7 +289,7 @@ def test_shell_cmd_inputspec_1(plugin, results_function):
 
 
 @pytest.mark.parametrize("results_function", [result_no_submitter, result_submitter])
-def test_shell_cmd_inputspec_2(plugin, results_function):
+def test_shell_cmd_inputspec_2(plugin, results_function, use_validator):
     """ a command with executable, args and two command options,
         using a customized input_spec to add the opt to the command
         in the right place that is specified in metadata["cmd_pos"]
@@ -1513,6 +1512,50 @@ def test_shell_cmd_inputspec_state_1(plugin, results_function):
     assert res[1].output.stdout == "hi\n"
 
 
+def test_shell_cmd_inputspec_typeval_1(use_validator):
+    """ customized input_spec with a type that doesn't match the value
+     - raise an exception
+    """
+    cmd_exec = "echo"
+
+    my_input_spec = SpecInfo(
+        name="Input",
+        fields=[
+            (
+                "text",
+                attr.ib(
+                    type=int,
+                    metadata={"position": 1, "argstr": "", "help_string": "text"},
+                ),
+            )
+        ],
+        bases=(ShellSpec,),
+    )
+
+    with pytest.raises(TypeError):
+        shelly = ShellCommandTask(
+            executable=cmd_exec, text="hello", input_spec=my_input_spec
+        )
+
+
+def test_shell_cmd_inputspec_typeval_2(use_validator):
+    """ customized input_spec (shorter syntax) with a type that doesn't match the value
+     - raise an exception
+    """
+    cmd_exec = "echo"
+
+    my_input_spec = SpecInfo(
+        name="Input",
+        fields=[("text", int, {"position": 1, "argstr": "", "help_string": "text"})],
+        bases=(ShellSpec,),
+    )
+
+    with pytest.raises(TypeError):
+        shelly = ShellCommandTask(
+            executable=cmd_exec, text="hello", input_spec=my_input_spec
+        )
+
+
 @pytest.mark.parametrize("results_function", [result_no_submitter, result_submitter])
 def test_shell_cmd_inputspec_state_1a(plugin, results_function):
     """  adding state to the input from input_spec