Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
0487e5c
Initialize _label to a string
liamhuber Jan 10, 2025
11716f7
Hint the delimiter
liamhuber Jan 10, 2025
1d13f1c
Make SemanticParent a Generic
liamhuber Jan 10, 2025
265f6f8
Purge `ParentMost`
liamhuber Jan 10, 2025
a6654d6
Update comment
liamhuber Jan 10, 2025
ba1eb27
Use generic type
liamhuber Jan 10, 2025
619d936
Don't use generic in static method
liamhuber Jan 10, 2025
39083ef
Jump through mypy hoops
liamhuber Jan 10, 2025
82f8370
Remove unused import
liamhuber Jan 10, 2025
24947fa
Add dev note
liamhuber Jan 14, 2025
f0deac6
Merge branch 'mypy' into mypy_semantics
liamhuber Jan 14, 2025
c94e7e6
Remove HasParent
liamhuber Jan 13, 2025
94f11ca
Pull out static method
liamhuber Jan 14, 2025
52bdd79
Pull cyclicity check up to Semantic
liamhuber Jan 14, 2025
6c234ae
De-parent SemanticParent from Semantic
liamhuber Jan 14, 2025
ff25496
Remove redundant type check
liamhuber Jan 15, 2025
260714a
Give Semantic a generic parent type
liamhuber Jan 15, 2025
3221cb9
Remove unused import
liamhuber Jan 15, 2025
59a883d
Black
liamhuber Jan 15, 2025
48c86c4
Ruff sort imports
liamhuber Jan 15, 2025
35c5a58
Remove unused import
liamhuber Jan 15, 2025
72b87c4
Update docstrings
liamhuber Jan 15, 2025
a633dbf
Guarantee that semantic parents have a label
liamhuber Jan 15, 2025
1d20d08
:bug: don't assume parents have semantic_path
liamhuber Jan 15, 2025
799ccb2
Pull label default up into Semantic
liamhuber Jan 15, 2025
7ed0a38
Refactor: label validity check
liamhuber Jan 15, 2025
28b8a32
Refactor: rename class
liamhuber Jan 15, 2025
62c1da7
Add label restrictions
liamhuber Jan 15, 2025
c577dfe
Improve error messages
liamhuber Jan 15, 2025
0d7b2e4
Make SemanticParent a Generic
liamhuber Jan 10, 2025
127c2a7
Don't use generic in static method
liamhuber Jan 10, 2025
6d60ccc
Remove unused import
liamhuber Jan 10, 2025
e2958b9
Remove HasParent
liamhuber Jan 13, 2025
e50d37c
Pull out static method
liamhuber Jan 14, 2025
afa53d4
Pull cyclicity check up to Semantic
liamhuber Jan 14, 2025
d7e69ad
De-parent SemanticParent from Semantic
liamhuber Jan 14, 2025
1f32ee2
Remove redundant type check
liamhuber Jan 15, 2025
33d3ce7
Give Semantic a generic parent type
liamhuber Jan 15, 2025
a1815e5
Remove unused import
liamhuber Jan 15, 2025
9dd113d
Black
liamhuber Jan 15, 2025
6cb92c0
Ruff sort imports
liamhuber Jan 15, 2025
5cb5699
Remove unused import
liamhuber Jan 15, 2025
f516fe3
Update docstrings
liamhuber Jan 15, 2025
b9e651e
Merge branch 'semantics_generic_parent' into optional_semantic_label
liamhuber Jan 16, 2025
40d9144
Annotate some extra returns (#548)
liamhuber Jan 16, 2025
53771dc
Merge branch 'mypy' into optional_semantic_label
liamhuber Jan 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions pyiron_workflow/channels.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion pyiron_workflow/mixin/has_interface_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
68 changes: 41 additions & 27 deletions pyiron_workflow/mixin/semantics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)

Expand All @@ -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:
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -302,25 +316,25 @@ 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 "
f"{child.parent.label}. Please remove it there before trying to "
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:
Expand All @@ -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__():
Expand Down Expand Up @@ -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
):
Expand Down
5 changes: 1 addition & 4 deletions pyiron_workflow/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyiron_workflow/nodes/composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 1 addition & 3 deletions tests/unit/mixin/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 23 additions & 10 deletions tests/unit/mixin/test_semantics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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"
Expand Down
Loading