diff --git a/pyiron_workflow/channels.py b/pyiron_workflow/channels.py index 829708785..153932fd5 100644 --- a/pyiron_workflow/channels.py +++ b/pyiron_workflow/channels.py @@ -92,10 +92,6 @@ def __init__( self.owner: HasIO = owner self.connections: list[ConjugateType] = [] - @property - def label(self) -> str: - return self._label - @abstractmethod def __str__(self): pass diff --git a/pyiron_workflow/mixin/has_interface_mixins.py b/pyiron_workflow/mixin/has_interface_mixins.py index 729431761..5183c83e0 100644 --- a/pyiron_workflow/mixin/has_interface_mixins.py +++ b/pyiron_workflow/mixin/has_interface_mixins.py @@ -39,10 +39,24 @@ class HasLabel(ABC): A mixin to guarantee the label interface exists. """ + _label: str + @property - @abstractmethod def label(self) -> str: """A label for the object.""" + return self._label + + @label.setter + def label(self, new_label: str): + self._check_label(new_label) + self._label = new_label + + def _check_label(self, new_label: str) -> None: + """ + Extensible checking routine for label validity. + """ + if not isinstance(new_label, str): + raise TypeError(f"Expected a string label but got {new_label}") @property def full_label(self) -> str: diff --git a/pyiron_workflow/mixin/semantics.py b/pyiron_workflow/mixin/semantics.py index 8c1ef6d4a..1ecfd8fb5 100644 --- a/pyiron_workflow/mixin/semantics.py +++ b/pyiron_workflow/mixin/semantics.py @@ -17,7 +17,7 @@ from abc import ABC, abstractmethod from difflib import get_close_matches from pathlib import Path -from typing import Generic, TypeVar +from typing import ClassVar, Generic, TypeVar from bidict import bidict @@ -35,13 +35,19 @@ class Semantic(UsesState, HasLabel, Generic[ParentType], ABC): accessible. """ - semantic_delimiter: str = "/" + semantic_delimiter: ClassVar[str] = "/" - def __init__(self, label: str, *args, parent: ParentType | None = None, **kwargs): + def __init__( + self, + *args, + label: str | None = None, + parent: ParentType | None = None, + **kwargs, + ): self._label = "" self._parent = None self._detached_parent_path = None - self.label = label + self.label = self.__class__.__name__ if label is None else label self.parent = parent super().__init__(*args, **kwargs) @@ -50,17 +56,13 @@ def __init__(self, label: str, *args, parent: ParentType | None = None, **kwargs def parent_type(cls) -> type[ParentType]: pass - @property - def label(self) -> str: - return self._label - - @label.setter - def label(self, new_label: str) -> None: - if not isinstance(new_label, str): - raise TypeError(f"Expected a string label but got {new_label}") + def _check_label(self, new_label: str) -> None: + super()._check_label(new_label) if self.semantic_delimiter in new_label: - raise ValueError(f"{self.semantic_delimiter} cannot be in the label") - self._label = new_label + raise ValueError( + f"Semantic delimiter {self.semantic_delimiter} cannot be in new label " + f"{new_label}" + ) @property def parent(self) -> ParentType | None: @@ -104,18 +106,22 @@ def semantic_path(self) -> str: The path of node labels from the graph root (parent-most node) down to this node. """ + prefix: str if self.parent is None and self.detached_parent_path is None: prefix = "" elif self.parent is None and self.detached_parent_path is not None: prefix = self.detached_parent_path elif self.parent is not None and self.detached_parent_path is None: - prefix = self.parent.semantic_path + if isinstance(self.parent, Semantic): + prefix = self.parent.semantic_path + else: + prefix = self.semantic_delimiter + self.parent.label else: raise ValueError( f"The parent and detached path should not be able to take non-None " f"values simultaneously, but got {self.parent} and " - f"{self.detached_parent_path}, respectively. Please raise an issue on GitHub " - f"outlining how your reached this state." + f"{self.detached_parent_path}, respectively. Please raise an issue on " + f"GitHub outlining how your reached this state." ) return prefix + self.semantic_delimiter + self.label @@ -179,9 +185,9 @@ class CyclicPathError(ValueError): ChildType = TypeVar("ChildType", bound=Semantic) -class SemanticParent(Generic[ChildType], ABC): +class SemanticParent(HasLabel, Generic[ChildType], ABC): """ - An with a collection of uniquely-named semantic children. + A labeled object with a collection of uniquely-named semantic children. Children should be added or removed via the :meth:`add_child` and :meth:`remove_child` methods and _not_ by direct manipulation of the @@ -198,14 +204,13 @@ class SemanticParent(Generic[ChildType], ABC): def __init__( self, - label: str | None, # Vestigial while the label order is broken *args, strict_naming: bool = True, **kwargs, ): self._children: bidict[str, ChildType] = bidict() self.strict_naming = strict_naming - super().__init__(*args, label=label, **kwargs) + super().__init__(*args, **kwargs) @classmethod @abstractmethod @@ -225,6 +230,15 @@ def children(self) -> bidict[str, ChildType]: def child_labels(self) -> tuple[str]: return tuple(child.label for child in self) + def _check_label(self, new_label: str) -> None: + super()._check_label(new_label) + if self.child_type().semantic_delimiter in new_label: + raise ValueError( + f"Child type ({self.child_type()}) semantic delimiter " + f"{self.child_type().semantic_delimiter} cannot be in new label " + f"{new_label}" + ) + def __getattr__(self, key) -> ChildType: try: return self._children[key] @@ -302,7 +316,7 @@ def add_child( child.parent = self return child - def _ensure_child_has_no_other_parent(self, child: Semantic): + def _ensure_child_has_no_other_parent(self, child: Semantic) -> None: if child.parent is not None and child.parent is not self: raise ValueError( f"The child ({child.label}) already belongs to the parent " @@ -310,17 +324,17 @@ def _ensure_child_has_no_other_parent(self, child: Semantic): f"add it to this parent ({self.label})." ) - def _this_child_is_already_at_this_label(self, child: Semantic, label: str): + def _this_child_is_already_at_this_label(self, child: Semantic, label: str) -> bool: return ( label == child.label and label in self.child_labels and self.children[label] is child ) - def _this_child_is_already_at_a_different_label(self, child, label): + def _this_child_is_already_at_a_different_label(self, child, label) -> bool: return child.parent is self and label != child.label - def _get_unique_label(self, label: str, strict_naming: bool): + def _get_unique_label(self, label: str, strict_naming: bool) -> str: if label in self.__dir__(): if label in self.child_labels: if strict_naming: @@ -337,7 +351,7 @@ def _get_unique_label(self, label: str, strict_naming: bool): ) return label - def _add_suffix_to_label(self, label): + def _add_suffix_to_label(self, label: str) -> str: i = 0 new_label = label while new_label in self.__dir__(): @@ -402,7 +416,7 @@ def __setstate__(self, state): child.parent = self -def _ensure_path_is_not_cyclic(parent, child: Semantic): +def _ensure_path_is_not_cyclic(parent, child: Semantic) -> None: if isinstance(parent, Semantic) and parent.semantic_path.startswith( child.semantic_path + child.semantic_delimiter ): diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 6e19a7042..ead473f74 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -297,10 +297,7 @@ def __init__( **kwargs: Interpreted as node input data, with keys corresponding to channel labels. """ - super().__init__( - label=self.__class__.__name__ if label is None else label, - parent=parent, - ) + super().__init__(label=label, parent=parent) self.checkpoint = checkpoint self.recovery: Literal["pickle"] | StorageInterface | None = "pickle" self._serialize_result = False # Advertised, but private to indicate diff --git a/pyiron_workflow/nodes/composite.py b/pyiron_workflow/nodes/composite.py index e5e05ed4c..1689da924 100644 --- a/pyiron_workflow/nodes/composite.py +++ b/pyiron_workflow/nodes/composite.py @@ -143,8 +143,8 @@ def __init__( # empty but the running_children list is not super().__init__( - label, *args, + label=label, parent=parent, delete_existing_savefiles=delete_existing_savefiles, autoload=autoload, diff --git a/tests/unit/mixin/test_run.py b/tests/unit/mixin/test_run.py index 009e03b55..bfa32ece4 100644 --- a/tests/unit/mixin/test_run.py +++ b/tests/unit/mixin/test_run.py @@ -8,9 +8,7 @@ class ConcreteRunnable(Runnable): - @property - def label(self) -> str: - return "child_class_with_all_methods_implemented" + _label = "child_class_with_all_methods_implemented" def on_run(self, **kwargs): return kwargs diff --git a/tests/unit/mixin/test_semantics.py b/tests/unit/mixin/test_semantics.py index 874928f79..fbb9bac47 100644 --- a/tests/unit/mixin/test_semantics.py +++ b/tests/unit/mixin/test_semantics.py @@ -12,23 +12,29 @@ class ConcreteSemantic(Semantic["ConcreteParent"]): @classmethod - def parent_type(cls) -> type[ConcreteParent]: - return ConcreteParent + def parent_type(cls) -> type[ConcreteSemanticParent]: + return ConcreteSemanticParent -class ConcreteParent(SemanticParent[ConcreteSemantic], ConcreteSemantic): +class ConcreteParent(SemanticParent[ConcreteSemantic]): + _label = "concrete_parent_default_label" + @classmethod def child_type(cls) -> type[ConcreteSemantic]: return ConcreteSemantic +class ConcreteSemanticParent(ConcreteParent, ConcreteSemantic): + pass + + class TestSemantics(unittest.TestCase): def setUp(self): - self.root = ConcreteParent("root") - self.child1 = ConcreteSemantic("child1", parent=self.root) - self.middle1 = ConcreteParent("middle", parent=self.root) - self.middle2 = ConcreteParent("middle_sub", parent=self.middle1) - self.child2 = ConcreteSemantic("child2", parent=self.middle2) + self.root = ConcreteSemanticParent(label="root") + self.child1 = ConcreteSemantic(label="child1", parent=self.root) + self.middle1 = ConcreteSemanticParent(label="middle", parent=self.root) + self.middle2 = ConcreteSemanticParent(label="middle_sub", parent=self.middle1) + self.child2 = ConcreteSemantic(label="child2", parent=self.middle2) def test_getattr(self): with self.assertRaises(AttributeError) as context: @@ -55,7 +61,14 @@ def test_label_delimiter(self): ValueError, msg=f"Delimiter '{ConcreteSemantic.semantic_delimiter}' not allowed", ): - ConcreteSemantic(f"invalid{ConcreteSemantic.semantic_delimiter}label") + ConcreteSemantic(label=f"invalid{ConcreteSemantic.semantic_delimiter}label") + + non_semantic_parent = ConcreteParent() + with self.assertRaises( + ValueError, + msg=f"Delimiter '{ConcreteSemantic.semantic_delimiter}' not allowed", + ): + non_semantic_parent.label = f"contains_{non_semantic_parent.child_type().semantic_delimiter}_delimiter" def test_semantic_delimiter(self): self.assertEqual( @@ -114,7 +127,7 @@ def test_as_path(self): ) def test_detached_parent_path(self): - orphan = ConcreteSemantic("orphan") + orphan = ConcreteSemantic(label="orphan") orphan.__setstate__(self.child2.__getstate__()) self.assertIsNone( orphan.parent, msg="We still should not explicitly have a parent"