Skip to content

[patch] Detatched path #457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 44 additions & 12 deletions pyiron_workflow/mixin/semantics.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def __init__(
):
self._label = None
self._parent = None
self._detached_parent_path = None
self.label = label
self.parent = parent
super().__init__(*args, **kwargs)
Expand All @@ -64,17 +65,21 @@ def parent(self, new_parent: SemanticParent | None) -> None:
# Exit early if nothing is changing
return

if new_parent is not None:
if not isinstance(new_parent, SemanticParent):
raise ValueError(
f"Expected None or a {SemanticParent.__name__} for the parent of "
f"{self.label}, but got {new_parent}"
)
if new_parent is not None and not isinstance(new_parent, SemanticParent):
raise ValueError(
f"Expected None or a {SemanticParent.__name__} for the parent of "
f"{self.label}, but got {new_parent}"
)

if self._parent is not None and new_parent is not self._parent:
if (
self._parent is not None
and new_parent is not self._parent
and self in self._parent.children
):
self._parent.remove_child(self)
self._parent = new_parent
if self._parent is not None:
self._detached_parent_path = None
if self._parent is not None and self not in self._parent.children:
self._parent.add_child(self)

@property
Expand All @@ -83,9 +88,34 @@ def semantic_path(self) -> str:
The path of node labels from the graph root (parent-most node) down to this
node.
"""
prefix = self.parent.semantic_path if isinstance(self.parent, Semantic) else ""
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
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."
)
return prefix + self.semantic_delimiter + self.label

@property
def detached_parent_path(self) -> str | None:
"""
The get/set state cycle of :class:`Semantic` de-parents objects, but we may
still be interested in the semantic path -- e.g. if we `pickle` dump and load
the object we will lose parent information, but this will still hold what the
path _was_ before the orphaning process.

The detached path will get cleared if a new parent is set, but is otherwise
used as the root for the purposes of finding the semantic path.
"""
return self._detached_parent_path

@property
def full_label(self) -> str:
"""
Expand All @@ -109,6 +139,8 @@ def as_path(self, root: Path | str | None = None) -> Path:

def __getstate__(self):
state = super().__getstate__()
if self.parent is not None:
state["_detached_parent_path"] = self.parent.semantic_path
state["_parent"] = None
# Regarding removing parent from state:
# Basically we want to avoid recursion during (de)serialization; when the
Expand Down Expand Up @@ -240,7 +272,7 @@ def add_child(
# Finally, update label and reflexively form the parent-child relationship
child.label = label
self.children[child.label] = child
child._parent = self
child.parent = self
return child

@staticmethod
Expand Down Expand Up @@ -316,7 +348,7 @@ def remove_child(self, child: Semantic | str) -> Semantic:
f"{Semantic.__name__} but got {child}"
)

child._parent = None
child.parent = None

return child

Expand Down Expand Up @@ -361,7 +393,7 @@ def __setstate__(self, state):
# but rather can send just the requested object and its scope (semantic
# children). So, now return their parent to them:
for child in self:
child._parent = self
child.parent = self


class ParentMostError(TypeError):
Expand Down
1 change: 1 addition & 0 deletions pyiron_workflow/nodes/composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ def _parse_remotely_executed_self(self, other_self):
# Un-parent existing nodes before ditching them
for node in self:
node._parent = None
node._detached_parent_path = None
other_self.running = False # It's done now
state = self._get_state_from_remote_other(other_self)
self.__setstate__(state)
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/mixin/test_semantics.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,39 @@ def test_as_path(self):
msg="Path root"
)

def test_detached_parent_path(self):
orphan = Semantic("orphan")
orphan.__setstate__(self.child2.__getstate__())
self.assertIsNone(
orphan.parent,
msg="We still should not explicitly have a parent"
)
self.assertListEqual(
orphan.detached_parent_path.split(orphan.semantic_delimiter),
self.child2.semantic_path.split(orphan.semantic_delimiter)[:-1],
msg="Despite not having a parent, the detached path should store semantic "
"path info through the get/set state routine"
)
self.assertEqual(
orphan.semantic_path,
self.child2.semantic_path,
msg="The detached path should carry through to semantic path in the "
"absence of a parent"
)
orphan.label = "orphan" # Re-set label after getting state
orphan.parent = self.child2.parent
self.assertIsNone(
orphan.detached_parent_path,
msg="Detached paths aren't necessary and shouldn't co-exist with the "
"presence of a parent"
)
self.assertListEqual(
orphan.semantic_path.split(orphan.semantic_delimiter)[:-1],
self.child2.semantic_path.split(self.child2.semantic_delimiter)[:-1],
msg="Sanity check -- except for the now-different labels, we should be "
"recovering the usual semantic path on setting a parent."
)


if __name__ == '__main__':
unittest.main()
Loading