Merge pull request #66 from pyiron/value_validation

Value validation

pyiron_workflow/channels.py

@@ -235,15 +235,15 @@ class DataChannel(Channel, ABC):
     (In the future they may optionally have a storage history limit.)
     (In the future they may optionally have an ontological type.)
 
-    Note that for the sake of computational efficiency, assignments to the `value`
-    property are not type-checked; type-checking occurs only for connections where both
-    channels have a type hint, and when a value is being copied from another channel
-    with the `copy_value` method.
+    Note that type checking is performed on value updates. This is typically not super
+    expensive, but once you have a workflow you're happy with, you may wish to
+    deactivate `strict_hints` throughout the workflow for the sake of computational
+    efficiency during production runs.
 
     When type checking channel connections, we insist that the output type hint be
     _as or more specific_ than the input type hint, to ensure that the input always
     receives output of a type it expects. This behaviour can be disabled and all
-    connections allowed by setting `strict_connections = False` on the relevant input
+    connections allowed by setting `strict_hints = False` on the relevant input
     channel.
 
     For simple type hints like `int` or `str`, type hint comparison is trivial.
@@ -271,6 +271,20 @@ class DataChannel(Channel, ABC):
         hint a tuple with a mixture of fixed elements of fixed type, followed by an
         arbitrary elements of arbitrary type. This and other complex scenarios are not
         yet included in our test suite and behaviour is not guaranteed.
+
+    Attributes:
+        value: The actual data value held by the node.
+        label (str): The label for the channel.
+        node (pyiron_workflow.node.Node): The node to which this channel belongs.
+        default (typing.Any|None): The default value to initialize to.
+            (Default is the class `NotData`.)
+        type_hint (typing.Any|None): A type hint for values. (Default is None.)
+        strict_hints (bool): Whether to check new values, connections, and partners
+            when this node is a value receiver. This can potentially be expensive, so
+            consider deactivating strict hints everywhere for production runs. (Default
+            is True, raise exceptions when type hints get violated.)
+        value_receiver (pyiron_workflow.node.Node|None): Another node of the same class
+            whose value will always get updated when this node's value gets updated.
     """
 
     def __init__(
@@ -279,14 +293,16 @@ def __init__(
         node: Node,
         default: typing.Optional[typing.Any] = NotData,
         type_hint: typing.Optional[typing.Any] = None,
+        strict_hints: bool = True,
         value_receiver: typing.Optional[InputData] = None,
     ):
         super().__init__(label=label, node=node)
         self._value = NotData
         self._value_receiver = None
-        self.default = default
-        self.value = default
         self.type_hint = type_hint
+        self.strict_hints = strict_hints
+        self.default = default
+        self.value = default  # Implicitly type check your default by assignment
         self.value_receiver = value_receiver
 
     @property
@@ -295,6 +311,16 @@ def value(self):
 
     @value.setter
     def value(self, new_value):
+        if (
+            self.strict_hints
+            and new_value is not NotData
+            and self._has_hint
+            and not valid_value(new_value, self.type_hint)
+        ):
+            raise TypeError(
+                f"The channel {self.label} cannot take the value `{new_value}` because "
+                f"it is not compliant with the type hint {self.type_hint}"
+            )
         if self.value_receiver is not None:
             self.value_receiver.value = new_value
         self._value = new_value
@@ -324,6 +350,17 @@ def value_receiver(self, new_partner: InputData | OutputData | None):
                     f"{self.__class__.__name__} {self.label} cannot couple to itself"
                 )
 
+            if self._both_typed(new_partner) and new_partner.strict_hints:
+                if not type_hint_is_as_or_more_specific_than(
+                    self.type_hint, new_partner.type_hint
+                ):
+                    raise ValueError(
+                        f"The channel {self.label} cannot take {new_partner.label} as "
+                        f"a value receiver because this type hint ({self.type_hint}) "
+                        f"is not as or more specific than the receiving type hint "
+                        f"({new_partner.type_hint})."
+                    )
+
             new_partner.value = self.value
 
         self._value_receiver = new_partner
@@ -357,7 +394,7 @@ def _valid_connection(self, other) -> bool:
         if super()._valid_connection(other):
             if self._both_typed(other):
                 out, inp = self._figure_out_who_is_who(other)
-                if not inp.strict_connections:
+                if not inp.strict_hints:
                     return True
                 else:
                     return type_hint_is_as_or_more_specific_than(
@@ -378,59 +415,21 @@ def _figure_out_who_is_who(self, other: DataChannel) -> (OutputData, InputData):
     def __str__(self):
         return str(self.value)
 
-    def copy_value(self, other: DataChannel) -> None:
-        """
-        Copies the other channel's value. Unlike normal value assignment, the new value
-        (if it is data) must comply with this channel's type hint (if any).
-        """
-        if (
-            self._has_hint
-            and other._value_is_data
-            and not valid_value(other.value, self.type_hint)
-        ):
-            raise TypeError(
-                f"Channel{self.label} cannot copy value from {other.label} because "
-                f"value {other.value} does not match type hint {self.type_hint}"
-            )
-        self.value = other.value
-
     def to_dict(self) -> dict:
         d = super().to_dict()
         d["value"] = repr(self.value)
         d["ready"] = self.ready
         d["type_hint"] = str(self.type_hint)
         return d
 
+    def activate_strict_hints(self) -> None:
+        self.strict_hints = True
 
-class InputData(DataChannel):
-    """
-    `fetch()` updates input data value to the first available data among connections.
-
-    The `strict_connections` parameter controls whether connections are subject to
-    type checking requirements.
-    I.e., they may set `strict_connections` to `False` (`True` -- default) at
-    instantiation or later with `(de)activate_strict_connections()` to prevent (enable)
-    data type checking when making connections with `OutputData` channels.
-    """
+    def deactivate_strict_hints(self) -> None:
+        self.strict_hints = False
 
-    def __init__(
-        self,
-        label: str,
-        node: Node,
-        default: typing.Optional[typing.Any] = NotData,
-        type_hint: typing.Optional[typing.Any] = None,
-        value_receiver: typing.Optional[InputData] = None,
-        strict_connections: bool = True,
-    ):
-        super().__init__(
-            label=label,
-            node=node,
-            default=default,
-            type_hint=type_hint,
-            value_receiver=value_receiver,
-        )
-        self.strict_connections = strict_connections
 
+class InputData(DataChannel):
     def fetch(self) -> None:
         """
         Sets `value` to the first value among connections that is something other than
@@ -451,12 +450,6 @@ def fetch(self) -> None:
                 self.value = out.value
                 break
 
-    def activate_strict_connections(self) -> None:
-        self.strict_connections = True
-
-    def deactivate_strict_connections(self) -> None:
-        self.strict_connections = False
-
 
 class OutputData(DataChannel):
     pass

pyiron_workflow/composite.py

@@ -126,6 +126,16 @@ def outputs_map(self, new_map: dict | bidict | None):
         new_map = new_map if new_map is None else bidict(new_map)
         self._outputs_map = new_map
 
+    def activate_strict_hints(self):
+        super().activate_strict_hints()
+        for node in self.nodes:
+            node.activate_strict_hints()
+
+    def deactivate_strict_hints(self):
+        super().deactivate_strict_hints()
+        for node in self.nodes:
+            node.deactivate_strict_hints()
+
     @property
     def _owned_creator(self):
         """

pyiron_workflow/function.py

@@ -157,8 +157,7 @@ class Function(Node):
         Note that getting "good" (i.e. dot-accessible) output labels can be achieved by
         using good variable names and returning those variables instead of using
         `output_labels`.
-        If we force the node to run with bad types, it will raise an
-        error:
+        If we try to assign a value of the wrong type, it will raise an error:
         >>> from typing import Union
         >>>
         >>> def hinted_example(
@@ -168,7 +167,17 @@ class Function(Node):
         ...     p1, m1 = x+1, y-1
         ...     return p1, m1
         >>>
-        >>> plus_minus_1 = Function(hinted_example, x="not an int")
+        >>> plus_minus_1 = Function(hinted_example)
+        >>> plus_minus_1.inputs.x =  "not an int or float"
+        TypeError: The channel x cannot take the value `not an int or float` because it
+        is not compliant with the type hint typing.Union[int, float]
+
+        We can turn off type hinting with the `strict_hints` boolean property, or just
+        circumvent the type hinting by applying the new data directly to the private
+        `_value` property.
+        In the latter case, we'd still get a readiness error when we try to run and
+        the ready check sees that the data doesn't conform to the type hint:
+        >>> plus_minus_1.inputs.x._value =  "not an int or float"
         >>> plus_minus_1.run()
         ValueError: hinted_example received a run command but is not ready. The node
         should be neither running nor failed, and all input values should conform to
@@ -180,13 +189,9 @@ class Function(Node):
 
         Here, even though all the input has data, the node sees that some of it is the
         wrong type and so (by default) the run raises an error right away.
-        Note that the type hinting doesn't actually prevent us from assigning bad values
-        directly to the channel (although it will, by default, prevent connections
-        _between_ type-hinted channels with incompatible hints), but it _does_ stop the
-        node from running and throwing an error because it sees that the channel (and
-        thus node) is not ready
-        >>> plus_minus_1.inputs.x.value
-        'not an int'
+        This causes the failure to come earlier because we stop the node from running
+        and throwing an error because it sees that the channel (and thus node) is not
+        ready:
 
         >>> plus_minus_1.ready, plus_minus_1.inputs.x.ready, plus_minus_1.inputs.y.ready
         (False, False, True)

pyiron_workflow/io.py

@@ -191,18 +191,18 @@ def to_dict(self):
         d["ready"] = self.ready
         return d
 
+    def activate_strict_hints(self):
+        [c.activate_strict_hints() for c in self]
+
+    def deactivate_strict_hints(self):
+        [c.deactivate_strict_hints() for c in self]
+
 
 class Inputs(DataIO):
     @property
     def _channel_class(self) -> type(InputData):
         return InputData
 
-    def activate_strict_connections(self):
-        [c.activate_strict_connections() for c in self]
-
-    def deactivate_strict_connections(self):
-        [c.deactivate_strict_connections() for c in self]
-
     def fetch(self):
         for c in self:
             c.fetch()

pyiron_workflow/macro.py

@@ -205,9 +205,7 @@ def _get_linking_channel(
         composite_channel.value = child_reference_channel.value
 
         if isinstance(composite_channel, InputData):
-            composite_channel.strict_connections = (
-                child_reference_channel.strict_connections
-            )
+            composite_channel.strict_hints = child_reference_channel.strict_hints
             composite_channel.value_receiver = child_reference_channel
         elif isinstance(composite_channel, OutputData):
             child_reference_channel.value_receiver = composite_channel

pyiron_workflow/node.py

@@ -600,6 +600,16 @@ def draw(
         """
         return GraphvizNode(self, depth=depth, rankdir=rankdir).graph
 
+    def activate_strict_hints(self):
+        """Enable type hint checks for all data IO"""
+        self.inputs.activate_strict_hints()
+        self.outputs.activate_strict_hints()
+
+    def deactivate_strict_hints(self):
+        """Disable type hint checks for all data IO"""
+        self.inputs.deactivate_strict_hints()
+        self.outputs.deactivate_strict_hints()
+
     def __str__(self):
         return (
             f"{self.label} ({self.__class__.__name__}):\n"
@@ -760,7 +770,7 @@ def _copy_values(
                 if to_copy.value is not NotData:
                     try:
                         old_value = my_panel[key].value
-                        my_panel[key].copy_value(to_copy)
+                        my_panel[key].value = to_copy.value  # Gets hint-checked
                         old_values.append((my_panel[key], old_value))
                     except Exception as e:
                         if fail_hard: