New class hierarchy and better readiness checking

pyiron_contrib/workflow/has_nodes.py

@@ -20,6 +20,7 @@ class HasNodes(ABC):
     """
 
     def __init__(self, *args, strict_naming=True, **kwargs):
+        super().__init__(*args, **kwargs)
         self.nodes: DotDict = DotDict()
         self.add: NodeAdder = NodeAdder(self)
         self.strict_naming: bool = strict_naming
@@ -39,16 +40,16 @@ def add_node(self, node: Node, label: Optional[str] = None) -> None:
             raise TypeError(
                 f"Only new node instances may be added, but got {type(node)}."
             )
-
-        label = self._ensure_label_is_unique(node.label if label is None else label)
-        self._ensure_node_has_no_other_parent(node, label)
+        self._ensure_node_has_no_other_parent(node)
+        label = self._get_unique_label(node.label if label is None else label)
+        self._ensure_node_is_not_duplicated(node, label)
 
         self.nodes[label] = node
         node.label = label
         node.parent = self
         return node
 
-    def _ensure_label_is_unique(self, label):
+    def _get_unique_label(self, label):
         if label in self.__dir__():
             if isinstance(getattr(self, label), Node):
                 if self.strict_naming:
@@ -78,23 +79,25 @@ def _add_suffix_to_label(self, label):
             )
         return new_label
 
-    def _ensure_node_has_no_other_parent(self, node: Node, label: str):
+    def _ensure_node_has_no_other_parent(self, node: Node):
+        if node.parent is not None and node.parent is not self:
+            raise ValueError(
+                f"The node ({node.label}) already belongs to the parent "
+                f"{node.parent.label}. Please remove it there before trying to "
+                f"add it to this parent ({self.label})."
+            )
+
+    def _ensure_node_is_not_duplicated(self, node: Node, label: str):
         if (
-            node.parent is self  # This should guarantee the node is in self.nodes
+            node.parent is self
             and label != node.label
+            and self.nodes[node.label] is node
         ):
-            assert self.nodes[node.label] is node  # Should be unreachable by users
             warn(
                 f"Reassigning the node {node.label} to the label {label} when "
                 f"adding it to the parent {self.label}."
             )
             del self.nodes[node.label]
-        elif node.parent is not None:
-            raise ValueError(
-                f"The node ({node.label}) already belongs to the parent "
-                f"{node.parent.label}. Please remove it there before trying to "
-                f"add it to this parent ({self.label})."
-            )
 
     def remove(self, node: Node | str):
         if isinstance(node, Node):

pyiron_contrib/workflow/is_nodal.py

@@ -0,0 +1,81 @@
+from __future__ import annotations
+
+from abc import ABC, abstractmethod
+from typing import TYPE_CHECKING
+
+from pyiron_contrib.workflow.io import Signals, InputSignal, OutputSignal
+
+if TYPE_CHECKING:
+    from pyiron_base.jobs.job.extension.server.generic import Server
+
+    from pyiron_contrib.workflow.io import Inputs, Outputs
+
+
+class IsNodal(ABC):
+    """
+    A mixin class for classes that can represent nodes on a computation graph.
+    """
+
+    def __init__(self, label: str, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.label: str = label
+        self.running = False
+        self.failed = False
+        # TODO: Replace running and failed with a state object
+        self._server: Server | None = (
+            None  # Or "task_manager" or "executor" -- we'll see what's best
+        )
+        self.signals = self._build_signal_channels()
+
+    @property
+    @abstractmethod
+    def inputs(self) -> Inputs:
+        pass
+
+    @property
+    @abstractmethod
+    def outputs(self) -> Outputs:
+        pass
+
+    @abstractmethod
+    def update(self):
+        pass
+
+    @abstractmethod
+    def run(self):
+        pass
+
+    def _build_signal_channels(self) -> Signals:
+        signals = Signals()
+        signals.input.run = InputSignal("run", self, self.run)
+        signals.output.ran = OutputSignal("ran", self)
+        return signals
+
+    @property
+    def server(self) -> Server | None:
+        return self._server
+
+    @server.setter
+    def server(self, server: Server | None):
+        self._server = server
+
+    def disconnect(self):
+        self.inputs.disconnect()
+        self.outputs.disconnect()
+        self.signals.disconnect()
+
+    @property
+    def ready(self) -> bool:
+        return not (self.running or self.failed) and self.inputs.ready
+
+    @property
+    def connected(self) -> bool:
+        return self.inputs.connected or self.outputs.connected or self.signals.connected
+
+    @property
+    def fully_connected(self):
+        return (
+            self.inputs.fully_connected
+            and self.outputs.fully_connected
+            and self.signals.fully_connected
+        )

pyiron_contrib/workflow/node.py

@@ -5,21 +5,17 @@
 from functools import partialmethod
 from typing import get_args, get_type_hints, Optional, TYPE_CHECKING
 
-from pyiron_contrib.workflow.channels import (
-    InputData,
-    OutputData,
-    InputSignal,
-    OutputSignal,
-)
+from pyiron_contrib.workflow.channels import InputData, OutputData
 from pyiron_contrib.workflow.has_channel import HasChannel
 from pyiron_contrib.workflow.has_to_dict import HasToDict
 from pyiron_contrib.workflow.io import Inputs, Outputs, Signals
+from pyiron_contrib.workflow.is_nodal import IsNodal
 
 if TYPE_CHECKING:
     from pyiron_contrib.workflow.workflow import Workflow
 
 
-class Node(HasToDict):
+class Node(IsNodal, HasToDict):
     """
     Nodes have input and output data channels that interface with the outside world, and
     a callable that determines what they actually compute. After running, their output
@@ -323,28 +319,25 @@ def __init__(
         parent: Optional[Workflow] = None,
         **kwargs,
     ):
+        super().__init__(
+            label=label if label is not None else node_function.__name__,
+            # **kwargs,
+        )
+        self.parent = parent
+        if parent is not None:
+            parent.add(self)
         if len(output_labels) == 0:
             raise ValueError("Nodes must have at least one output label.")
 
-        self.running = False
-        self.failed = False
-        self.server = None  # Or "task_manager" or "executor" -- we'll see what's best
         self.node_function = node_function
-        self.label = label if label is not None else node_function.__name__
-
-        self.parent = None
-        if parent is not None:
-            parent.add(self)
 
         input_channels = self._build_input_channels(input_storage_priority)
-        self.inputs = Inputs(*input_channels)
+        self._inputs = Inputs(*input_channels)
 
         output_channels = self._build_output_channels(
             *output_labels, storage_priority=output_storage_priority
         )
-        self.outputs = Outputs(*output_channels)
-
-        self.signals = self._build_signal_channels()
+        self._outputs = Outputs(*output_channels)
 
         self.channels_requiring_update_after_run = (
             []
@@ -364,6 +357,14 @@ def __init__(
         if update_on_instantiation:
             self.update()
 
+    @property
+    def inputs(self) -> Inputs:
+        return self._inputs
+
+    @property
+    def outputs(self) -> Outputs:
+        return self._outputs
+
     def _build_input_channels(self, storage_priority: dict[str:int]):
         channels = []
         type_hints = get_type_hints(self.node_function)
@@ -451,12 +452,6 @@ def _build_output_channels(
 
         return channels
 
-    def _build_signal_channels(self) -> Signals:
-        signals = Signals()
-        signals.input.run = InputSignal("run", self, self.run)
-        signals.output.ran = OutputSignal("ran", self)
-        return signals
-
     def _verify_that_channels_requiring_update_all_exist(self):
         if not all(
             channel_name in self.inputs.labels
@@ -520,27 +515,6 @@ def process_output(self, function_output):
     def __call__(self) -> None:
         self.run()
 
-    def disconnect(self):
-        self.inputs.disconnect()
-        self.outputs.disconnect()
-        self.signals.disconnect()
-
-    @property
-    def ready(self) -> bool:
-        return not (self.running or self.failed) and self.inputs.ready
-
-    @property
-    def connected(self) -> bool:
-        return self.inputs.connected or self.outputs.connected or self.signals.connected
-
-    @property
-    def fully_connected(self):
-        return (
-            self.inputs.fully_connected
-            and self.outputs.fully_connected
-            and self.signals.fully_connected
-        )
-
     def set_storage_priority(self, priority: int):
         self.inputs.set_storage_priority(priority)
         self.outputs.set_storage_priority(priority)

pyiron_contrib/workflow/workflow.py

@@ -2,6 +2,7 @@
 
 from pyiron_contrib.workflow.has_nodes import HasNodes
 from pyiron_contrib.workflow.has_to_dict import HasToDict
+from pyiron_contrib.workflow.is_nodal import IsNodal
 from pyiron_contrib.workflow.node import Node, node, fast_node, single_value_node
 from pyiron_contrib.workflow.util import DotDict
 
@@ -14,7 +15,7 @@ class _NodeDecoratorAccess:
     single_value_node = single_value_node
 
 
-class Workflow(HasToDict, HasNodes):
+class Workflow(IsNodal, HasToDict, HasNodes):
     """
     Workflows are an abstraction for holding a collection of related nodes.
 
@@ -119,8 +120,7 @@ class Workflow(HasToDict, HasNodes):
     wrap_as = _NodeDecoratorAccess
 
     def __init__(self, label: str, *nodes: Node, strict_naming=True):
-        super().__init__(strict_naming=strict_naming)
-        self.label = label
+        super().__init__(label=label, strict_naming=strict_naming)
 
         for node in nodes:
             self.add_node(node)

tests/unit/workflow/test_workflow.py

@@ -19,8 +19,14 @@ def test_node_addition(self):
         wf.add(Node(fnc, "x", label="foo"))
         wf.add.Node(fnc, "y", label="bar")
         wf.baz = Node(fnc, "y", label="whatever_baz_gets_used")
-        Node(fnc, "x", label="boa", parent=wf)
-        self.assertListEqual(list(wf.nodes.keys()), ["foo", "bar", "baz", "boa"])
+        Node(fnc, "x", label="qux", parent=wf)
+        self.assertListEqual(list(wf.nodes.keys()), ["foo", "bar", "baz", "qux"])
+        wf.boa = wf.qux
+        self.assertListEqual(
+            list(wf.nodes.keys()),
+            ["foo", "bar", "baz", "boa"],
+            msg="Reassignment should remove the original instance"
+        )
 
         wf.strict_naming = False
         # Validate name incrementation