Merge pull request #201 from djarecka/mnt/undefined_input

using attr.NOTHING for undefined input fields

Author: djarecka

pydra/engine/core.py

@@ -132,9 +132,7 @@ def __init__(
         # todo should be used to input_check in spec??
         self.inputs = klass(
             **{
-                (f.name[1:] if f.name.startswith("_") else f.name): (
-                    None if f.default == attr.NOTHING else f.default
-                )
+                (f.name[1:] if f.name.startswith("_") else f.name): f.default
                 for f in attr.fields(klass)
             }
         )
@@ -220,7 +218,7 @@ def version(self):
     def checksum(self):
         """Calculate a unique checksum of this task."""
         # if checksum is called before run the _graph_checksums is not ready
-        if is_workflow(self) and self.inputs._graph_checksums is None:
+        if is_workflow(self) and self.inputs._graph_checksums is attr.NOTHING:
             self.inputs._graph_checksums = [nd.checksum for nd in self.graph_sorted]
 
         input_hash = self.inputs.hash

pydra/engine/helpers_file.py

@@ -494,7 +494,7 @@ def template_update(inputs, map_copyfiles=None):
     fields = attr_fields(inputs)
     # TODO: Create a dependency graph first and then traverse it
     for fld in fields:
-        if getattr(inputs, fld.name) is not None:
+        if getattr(inputs, fld.name) is not attr.NOTHING:
             continue
         if fld.metadata.get("output_file_template"):
             if fld.type is str:

pydra/engine/specs.py

@@ -47,6 +47,9 @@ def hash(self):
                 "output_file_template"
             ):
                 continue
+            # removing values that are notset from hash calculation
+            if getattr(self, field.name) is attr.NOTHING:
+                continue
             inp_dict[field.name] = hash_value(
                 value=getattr(self, field.name), tp=field.type, metadata=field.metadata
             )
@@ -241,7 +244,7 @@ def check_fields_input_spec(self):
         for fld in fields:
             mdata = fld.metadata
             # checking if the mandatory field is provided
-            if getattr(self, fld.name) is None:
+            if getattr(self, fld.name) is attr.NOTHING:
                 if mdata.get("mandatory"):
                     raise Exception(f"{fld.name} is mandatory, but no value provided")
                 else:
@@ -370,7 +373,9 @@ def _field_metadata(self, fld, inputs, output_dir):
 class ContainerSpec(ShellSpec):
     """Refine the generic command-line specification to container execution."""
 
-    image: ty.Union[File, str] = attr.ib(metadata={"help_string": "image"})
+    image: ty.Union[File, str] = attr.ib(
+        metadata={"help_string": "image", "mandatory": True}
+    )
     """The image to be containerized."""
     container: ty.Union[File, str, None] = attr.ib(
         metadata={"help_string": "container"}

pydra/engine/task.py

@@ -463,7 +463,7 @@ def container_check(self, container_type):
             raise AttributeError(
                 f"Container type should be {container_type}, but {self.inputs.container} given"
             )
-        if self.inputs.image is None:
+        if self.inputs.image is attr.NOTHING:
             raise AttributeError("Container image is not specified")
 
     def bind_paths(self, ind=None):

pydra/engine/tests/test_node_task.py

@@ -1,11 +1,14 @@
 import os
 import shutil
+import attr
 import numpy as np
 import pytest
 
 from .utils import (
     fun_addtwo,
     fun_addvar,
+    fun_addvar_none,
+    fun_addvar_default,
     moment,
     fun_div,
     fun_dict,
@@ -317,6 +320,23 @@ def test_task_init_7(plugin, tmpdir):
     assert output_dir1.name != output_dir2.name
 
 
+def test_task_init_8():
+    """ task without setting the input, the value should be set to attr.NOTHING"""
+    nn = fun_addtwo(name="NA")
+    assert nn.inputs.a is attr.NOTHING
+
+
+def test_task_init_9():
+    """ task without setting the input, but using the default avlue from function"""
+    nn1 = fun_addvar_default(name="NA", a=2)
+    assert nn1.inputs.b == 1
+
+    nn2 = fun_addvar_default(name="NA", a=2, b=1)
+    assert nn2.inputs.b == 1
+    # both tasks should have the same checksum
+    assert nn1.checksum == nn2.checksum
+
+
 def test_task_error():
     func = fun_div(name="div", a=1, b=0)
     with pytest.raises(ZeroDivisionError):
@@ -474,6 +494,31 @@ def test_task_nostate_5(plugin, tmpdir):
     assert nn.output_dir.exists()
 
 
+def test_task_nostate_6():
+    """ checking if the function gets the None value"""
+    nn = fun_addvar_none(name="NA", a=2, b=None)
+    assert nn.inputs.b is None
+    nn()
+    assert nn.result().output.out == 2
+
+
+def test_task_nostate_6a_exception():
+    """ checking if the function gets the attr.Nothing value"""
+    nn = fun_addvar_none(name="NA", a=2)
+    assert nn.inputs.b is attr.NOTHING
+    with pytest.raises(TypeError) as excinfo:
+        nn()
+    assert "unsupported" in str(excinfo.value)
+
+
+def test_task_nostate_7():
+    """ using the default value from the function for b input"""
+    nn = fun_addvar_default(name="NA", a=2)
+    assert nn.inputs.b == 1
+    nn()
+    assert nn.result().output.out == 3
+
+
 # Testing caching for tasks without states
 
 

pydra/engine/tests/test_shelltask.py

@@ -458,6 +458,7 @@ def test_shell_cmd_inputspec_3c(plugin, results_function):
                 "text",
                 attr.ib(
                     type=str,
+                    default=None,
                     metadata={"position": 1, "help_string": "text", "mandatory": False},
                 ),
             )

pydra/engine/tests/test_task.py

@@ -352,11 +352,9 @@ def test_shell_cmd(tmpdir):
 
 def test_container_cmds(tmpdir):
     containy = DockerTask(name="containy", executable="pwd")
-    with pytest.raises(AttributeError):
-        containy.cmdline
-    containy.inputs.container = "docker"
-    with pytest.raises(AttributeError):
+    with pytest.raises(AttributeError) as excinfo:
         containy.cmdline
+    assert "not specified" in str(excinfo.value)
     containy.inputs.image = "busybox"
     assert containy.cmdline
 

pydra/engine/tests/test_workflow.py

@@ -1,7 +1,7 @@
 import pytest
 import shutil, os
 import time
-import platform
+import attr
 
 from .utils import (
     add2,
@@ -12,11 +12,11 @@
     list_output,
     fun_addvar3,
     add2_sub2_res,
+    fun_addvar_none,
+    fun_addvar_default,
 )
 from ..submitter import Submitter
 from ..core import Workflow
-from ... import mark
-
 
 if bool(shutil.which("sbatch")):
     Plugins = ["cf", "slurm"]
@@ -229,6 +229,84 @@ def test_wf_2d_outpasdict(plugin):
     assert wf.output_dir.exists()
 
 
+@pytest.mark.parametrize("plugin", Plugins)
+def test_wf_3(plugin):
+    """ testing None value for an input"""
+    wf = Workflow(name="wf_3", input_spec=["x", "y"])
+    wf.add(fun_addvar_none(name="addvar", a=wf.lzin.x, b=wf.lzin.y))
+    wf.add(add2(name="add2", x=wf.addvar.lzout.out))
+    wf.set_output([("out", wf.add2.lzout.out)])
+    wf.inputs.x = 2
+    wf.inputs.y = None
+    wf.plugin = plugin
+
+    with Submitter(plugin=plugin) as sub:
+        sub(wf)
+
+    assert wf.output_dir.exists()
+    results = wf.result()
+    assert 4 == results.output.out
+
+
+@pytest.mark.xfail(reason="the task error doesn't propagate")
+@pytest.mark.parametrize("plugin", Plugins)
+def test_wf_3a_exception(plugin):
+    """ testinh wf without set input, attr.NOTHING should be set
+        and the function should raise an exception
+    """
+    wf = Workflow(name="wf_3", input_spec=["x", "y"])
+    wf.add(fun_addvar_none(name="addvar", a=wf.lzin.x, b=wf.lzin.y))
+    wf.add(add2(name="add2", x=wf.addvar.lzout.out))
+    wf.set_output([("out", wf.add2.lzout.out)])
+    wf.inputs.x = 2
+    wf.inputs.y = attr.NOTHING
+    wf.plugin = plugin
+
+    with pytest.raises(TypeError) as excinfo:
+        with Submitter(plugin=plugin) as sub:
+            sub(wf)
+    assert "unsupported" in str(excinfo.value)
+
+
+@pytest.mark.parametrize("plugin", Plugins)
+def test_wf_4(plugin):
+    """wf with a task that doesn't set one input and use the function default value"""
+    wf = Workflow(name="wf_4", input_spec=["x", "y"])
+    wf.add(fun_addvar_default(name="addvar", a=wf.lzin.x))
+    wf.add(add2(name="add2", x=wf.addvar.lzout.out))
+    wf.set_output([("out", wf.add2.lzout.out)])
+    wf.inputs.x = 2
+    wf.plugin = plugin
+
+    with Submitter(plugin=plugin) as sub:
+        sub(wf)
+
+    assert wf.output_dir.exists()
+    results = wf.result()
+    assert 5 == results.output.out
+
+
+@pytest.mark.parametrize("plugin", Plugins)
+def test_wf_4a(plugin):
+    """ wf with a task that doesn't set one input,
+        the unset input is send to the task input,
+        so the task should use the function default value
+    """
+    wf = Workflow(name="wf_4a", input_spec=["x", "y"])
+    wf.add(fun_addvar_default(name="addvar", a=wf.lzin.x, y=wf.lzin.y))
+    wf.add(add2(name="add2", x=wf.addvar.lzout.out))
+    wf.set_output([("out", wf.add2.lzout.out)])
+    wf.inputs.x = 2
+    wf.plugin = plugin
+
+    with Submitter(plugin=plugin) as sub:
+        sub(wf)
+
+    assert wf.output_dir.exists()
+    results = wf.result()
+    assert 5 == results.output.out
+
+
 @pytest.mark.parametrize("plugin", Plugins)
 def test_wf_st_1(plugin):
     """ Workflow with one task, a splitter for the workflow"""

pydra/engine/tests/utils.py

@@ -21,6 +21,19 @@ def fun_addvar(a, b):
     return a + b
 
 
+@mark.task
+def fun_addvar_none(a, b):
+    if b is None:
+        return a
+    else:
+        return a + b
+
+
+@mark.task
+def fun_addvar_default(a, b=1):
+    return a + b
+
+
 @mark.task
 def fun_addvar3(a, b, c):
     return a + b + c