[patch] Detatched path (#457)

* [patch] carry semantic path information through get/set state process

* Refactor: handle the parent-child recursion more gracefully

So that semantic parents no longer need to be updating private attributes of their children to avoid the recursion. It was barely tolerable when it was a single attribute, but now that we also track the detached parent path it is totally unacceptable.

* Refactor: condense if clause

Author: liamhuber

pyiron_workflow/mixin/semantics.py

@@ -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)
@@ -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
@@ -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:
         """
@@ -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
@@ -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
@@ -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
 
@@ -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):

pyiron_workflow/nodes/composite.py

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

tests/unit/mixin/test_semantics.py

@@ -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()