From 197497cf20035b880215bb0ecd3d40c5cf4b07db Mon Sep 17 00:00:00 2001 From: liamhuber Date: Tue, 16 Jan 2024 12:51:53 -0800 Subject: [PATCH 01/17] Document the intended attribute --- pyiron_workflow/node.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index a4751550..133ea757 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -183,6 +183,7 @@ class Node(HasToDict, ABC, metaclass=AbstractHasPost): owning this, if any. ready (bool): Whether the inputs are all ready and the node is neither already running nor already failed. + root (Node): The parent-most node in this graph. run_args (dict): **Abstract** the argmuments to use for actually running the node. Must be specified in child classes. running (bool): Whether the node has called :meth:`run` and has not yet From 874aaff4a43f90035cc44630a4c2e916c7c674d2 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Tue, 16 Jan 2024 12:59:51 -0800 Subject: [PATCH 02/17] Implement and test the root property --- pyiron_workflow/node.py | 5 +++++ tests/unit/test_composite.py | 28 ++++++++++++++++++++++++++++ tests/unit/test_node.py | 8 ++++++++ 3 files changed, 41 insertions(+) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 133ea757..89285018 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -317,6 +317,11 @@ def parent(self, new_parent: Composite | None) -> None: "parent" ) + @property + def root(self) -> Node: + """The parent-most node in this graph.""" + return self if self.parent is None else self.parent.root + @property def readiness_report(self) -> str: input_readiness = "\n".join( diff --git a/tests/unit/test_composite.py b/tests/unit/test_composite.py index d5764f6d..b3750227 100644 --- a/tests/unit/test_composite.py +++ b/tests/unit/test_composite.py @@ -590,6 +590,34 @@ def test_de_activate_strict_connections(self): msg="Activating should propagate to children" ) + def test_root(self): + top = AComposite("topmost") + top.middle_composite = AComposite("middle_composite") + top.middle_composite.deep_node = Composite.create.SingleValue(plus_one) + top.middle_function = Composite.create.SingleValue(plus_one) + + self.assertIs( + top, + top.root, + msg="The parent-most node should be its own root." + ) + self.assertIs( + top, + top.middle_composite.root, + msg="The parent-most node should be the root." + ) + self.assertIs( + top, + top.middle_function.root, + msg="The parent-most node should be the root." + ) + self.assertIs( + top, + top.middle_composite.deep_node.root, + msg="The parent-most node should be the root, recursively accessible from " + "all depths." + ) + if __name__ == '__main__': unittest.main() diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py index 439eaf28..f17c6a8d 100644 --- a/tests/unit/test_node.py +++ b/tests/unit/test_node.py @@ -344,6 +344,14 @@ def test_run_after_init(self): msg="With run_after_init, the node should run right away" ) + def test_root(self): + n = ANode("n") + self.assertIs( + n, + n.root, + msg="Lone nodes should be their own root, as there is no parent above." + ) + if __name__ == '__main__': unittest.main() From 5b525f1db41e58726472d8fe8c3d28bf93502f7c Mon Sep 17 00:00:00 2001 From: liamhuber Date: Tue, 16 Jan 2024 14:06:36 -0800 Subject: [PATCH 03/17] Introduce a semantic path --- pyiron_workflow/node.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 89285018..213e4f41 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -225,6 +225,7 @@ class Node(HasToDict, ABC, metaclass=AbstractHasPost): """ package_identifier = None + _semantic_delimiter = "/" def __init__( self, @@ -322,6 +323,15 @@ def root(self) -> Node: """The parent-most node in this graph.""" return self if self.parent is None else self.parent.root + @property + def semantic_path(self): + path = self.label + if self.parent is not None: + path = self.parent.semantic_path + self._semantic_delimiter + path + # else: + # path = self.semantic_root + self._semantic_delimiter + path + return path + @property def readiness_report(self) -> str: input_readiness = "\n".join( From 1ec3f66d6c0783b943f58967c677f545cedcca36 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Tue, 16 Jan 2024 14:06:46 -0800 Subject: [PATCH 04/17] Don't let labels interfere with the path --- pyiron_workflow/node.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 213e4f41..1eeee680 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -247,6 +247,7 @@ def __init__( **kwargs: Keyword arguments passed on with `super`. """ super().__init__(*args, **kwargs) + self._label = None self.label: str = label self._parent = None if parent is not None: @@ -268,6 +269,16 @@ def __post__(self, *args, run_after_init: bool = False, **kwargs): except ReadinessError: pass + @property + def label(self) -> str: + return self._label + + @label.setter + def label(self, new_label: str): + if self._semantic_delimiter in new_label: + raise ValueError(f"{self._semantic_delimiter} cannot be in the label") + self._label = new_label + @property @abstractmethod def inputs(self) -> Inputs: From 9e0a9d604887e95a847fc8bfde9bdb8be89e8a9e Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:29:06 -0800 Subject: [PATCH 05/17] Use a more specific name We might have a storage root, etc. Also, the graph and the semantics are not identical, as multiple workflows might have a semantic (but not operational) interaction, so then we'd care about a semantic root! Plus it makes find-usages easier on the IDE when we have a more specific name. --- pyiron_workflow/node.py | 6 +++--- tests/unit/test_composite.py | 18 +++++++++--------- tests/unit/test_node.py | 5 +++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 89285018..0266936f 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -183,7 +183,7 @@ class Node(HasToDict, ABC, metaclass=AbstractHasPost): owning this, if any. ready (bool): Whether the inputs are all ready and the node is neither already running nor already failed. - root (Node): The parent-most node in this graph. + graph_root (Node): The parent-most node in this graph. run_args (dict): **Abstract** the argmuments to use for actually running the node. Must be specified in child classes. running (bool): Whether the node has called :meth:`run` and has not yet @@ -318,9 +318,9 @@ def parent(self, new_parent: Composite | None) -> None: ) @property - def root(self) -> Node: + def graph_root(self) -> Node: """The parent-most node in this graph.""" - return self if self.parent is None else self.parent.root + return self if self.parent is None else self.parent.graph_root @property def readiness_report(self) -> str: diff --git a/tests/unit/test_composite.py b/tests/unit/test_composite.py index b3750227..a9df414c 100644 --- a/tests/unit/test_composite.py +++ b/tests/unit/test_composite.py @@ -598,24 +598,24 @@ def test_root(self): self.assertIs( top, - top.root, - msg="The parent-most node should be its own root." + top.graph_root, + msg="The parent-most node should be its own graph_root." ) self.assertIs( top, - top.middle_composite.root, - msg="The parent-most node should be the root." + top.middle_composite.graph_root, + msg="The parent-most node should be the graph_root." ) self.assertIs( top, - top.middle_function.root, - msg="The parent-most node should be the root." + top.middle_function.graph_root, + msg="The parent-most node should be the graph_root." ) self.assertIs( top, - top.middle_composite.deep_node.root, - msg="The parent-most node should be the root, recursively accessible from " - "all depths." + top.middle_composite.deep_node.graph_root, + msg="The parent-most node should be the graph_root, recursively accessible " + "from all depths." ) diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py index f17c6a8d..7372b811 100644 --- a/tests/unit/test_node.py +++ b/tests/unit/test_node.py @@ -348,8 +348,9 @@ def test_root(self): n = ANode("n") self.assertIs( n, - n.root, - msg="Lone nodes should be their own root, as there is no parent above." + n.graph_root, + msg="Lone nodes should be their own graph_root, as there is no parent " + "above." ) From a08e11baa20ccc3d1b8261a5721a67c4c53240ce Mon Sep 17 00:00:00 2001 From: pyiron-runner Date: Thu, 18 Jan 2024 22:29:29 +0000 Subject: [PATCH 06/17] [dependabot skip] Update env file --- docs/environment.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/environment.yml b/docs/environment.yml index 13d3ebfe..3facc4ff 100644 --- a/docs/environment.yml +++ b/docs/environment.yml @@ -2,7 +2,10 @@ channels: - conda-forge dependencies: - ipykernel +- myst-parser - nbsphinx +- sphinx-gallery +- sphinx-rtd-theme - coveralls - coverage - bidict =0.22.1 @@ -13,4 +16,3 @@ dependencies: - python-graphviz =0.20.1 - toposort =1.10 - typeguard =4.1.5 -- myst-parser From 52c0049ed71445519c17114e0192335cd8ac98c8 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Tue, 16 Jan 2024 14:06:36 -0800 Subject: [PATCH 07/17] Introduce a semantic path --- pyiron_workflow/node.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 0266936f..666fcb89 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -225,6 +225,7 @@ class Node(HasToDict, ABC, metaclass=AbstractHasPost): """ package_identifier = None + _semantic_delimiter = "/" def __init__( self, @@ -322,6 +323,15 @@ def graph_root(self) -> Node: """The parent-most node in this graph.""" return self if self.parent is None else self.parent.graph_root + @property + def semantic_path(self): + path = self.label + if self.parent is not None: + path = self.parent.semantic_path + self._semantic_delimiter + path + # else: + # path = self.semantic_root + self._semantic_delimiter + path + return path + @property def readiness_report(self) -> str: input_readiness = "\n".join( From 9d4778d5ebecd728f7e2ff30a69075506055978a Mon Sep 17 00:00:00 2001 From: liamhuber Date: Tue, 16 Jan 2024 14:06:46 -0800 Subject: [PATCH 08/17] Don't let labels interfere with the path --- pyiron_workflow/node.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 666fcb89..ec3d3b84 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -247,6 +247,7 @@ def __init__( **kwargs: Keyword arguments passed on with `super`. """ super().__init__(*args, **kwargs) + self._label = None self.label: str = label self._parent = None if parent is not None: @@ -268,6 +269,16 @@ def __post__(self, *args, run_after_init: bool = False, **kwargs): except ReadinessError: pass + @property + def label(self) -> str: + return self._label + + @label.setter + def label(self, new_label: str): + if self._semantic_delimiter in new_label: + raise ValueError(f"{self._semantic_delimiter} cannot be in the label") + self._label = new_label + @property @abstractmethod def inputs(self) -> Inputs: From 04cd2be5f7503dd6ad5488f21150e197b07ff096 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:31:30 -0800 Subject: [PATCH 09/17] Refactor: rename Because this stops at the graph root, it's only _part_ of the semantic path, which might include super-workflow semantic objects. Rename accordingly --- pyiron_workflow/node.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index ec3d3b84..e9f6517c 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -335,10 +335,10 @@ def graph_root(self) -> Node: return self if self.parent is None else self.parent.graph_root @property - def semantic_path(self): + def graph_path(self): path = self.label if self.parent is not None: - path = self.parent.semantic_path + self._semantic_delimiter + path + path = self.parent.graph_path + self._semantic_delimiter + path # else: # path = self.semantic_root + self._semantic_delimiter + path return path From 34ea37bb07c2b46d8a133bddd3fff523e79b0f7f Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:36:18 -0800 Subject: [PATCH 10/17] Remove usage hint comment This is now _only_ the path inside the scope of the graph, so we no longer need to worry about a semantic root --- pyiron_workflow/node.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index e9f6517c..936a8fe9 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -339,8 +339,6 @@ def graph_path(self): path = self.label if self.parent is not None: path = self.parent.graph_path + self._semantic_delimiter + path - # else: - # path = self.semantic_root + self._semantic_delimiter + path return path @property From e2c07e9a9b61c589f7caeeb55218a68497a7d432 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Tue, 16 Jan 2024 14:06:36 -0800 Subject: [PATCH 11/17] Introduce a semantic path --- pyiron_workflow/node.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 936a8fe9..50d2303e 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -341,6 +341,15 @@ def graph_path(self): path = self.parent.graph_path + self._semantic_delimiter + path return path + @property + def semantic_path(self): + path = self.label + if self.parent is not None: + path = self.parent.semantic_path + self._semantic_delimiter + path + # else: + # path = self.semantic_root + self._semantic_delimiter + path + return path + @property def readiness_report(self) -> str: input_readiness = "\n".join( From 52d85d281f63d548a75a7b17aa1c0c7210572873 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:38:11 -0800 Subject: [PATCH 12/17] Remove junk from pulling remote --- pyiron_workflow/node.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 50d2303e..936a8fe9 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -341,15 +341,6 @@ def graph_path(self): path = self.parent.graph_path + self._semantic_delimiter + path return path - @property - def semantic_path(self): - path = self.label - if self.parent is not None: - path = self.parent.semantic_path + self._semantic_delimiter + path - # else: - # path = self.semantic_root + self._semantic_delimiter + path - return path - @property def readiness_report(self) -> str: input_readiness = "\n".join( From 9d401254cc0eb092041c31df80076af6bdc7a422 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:39:59 -0800 Subject: [PATCH 13/17] Add attribute to docstring --- pyiron_workflow/node.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 936a8fe9..2a815de3 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -183,6 +183,8 @@ class Node(HasToDict, ABC, metaclass=AbstractHasPost): owning this, if any. ready (bool): Whether the inputs are all ready and the node is neither already running nor already failed. + graph_path (str): The file-path-like path of node labels from the parent-most + node down to this node. graph_root (Node): The parent-most node in this graph. run_args (dict): **Abstract** the argmuments to use for actually running the node. Must be specified in child classes. @@ -335,7 +337,11 @@ def graph_root(self) -> Node: return self if self.parent is None else self.parent.graph_root @property - def graph_path(self): + def graph_path(self) -> str: + """ + The path of node labels from the graph root (parent-most node) down to this + node. + """ path = self.label if self.parent is not None: path = self.parent.graph_path + self._semantic_delimiter + path From 7288d688b0013a6716da3868753e2e2601f79b3c Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:44:23 -0800 Subject: [PATCH 14/17] Refactor: slide --- pyiron_workflow/node.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 2a815de3..e30ebd0d 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -331,11 +331,6 @@ def parent(self, new_parent: Composite | None) -> None: "parent" ) - @property - def graph_root(self) -> Node: - """The parent-most node in this graph.""" - return self if self.parent is None else self.parent.graph_root - @property def graph_path(self) -> str: """ @@ -347,6 +342,11 @@ def graph_path(self) -> str: path = self.parent.graph_path + self._semantic_delimiter + path return path + @property + def graph_root(self) -> Node: + """The parent-most node in this graph.""" + return self if self.parent is None else self.parent.graph_root + @property def readiness_report(self) -> str: input_readiness = "\n".join( From 75cb881cc618ee1a6abe94ab911d225e96b0364f Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:50:35 -0800 Subject: [PATCH 15/17] Add tests --- tests/unit/test_composite.py | 76 ++++++++++++++++++++++++++---------- tests/unit/test_node.py | 8 ++++ 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/tests/unit/test_composite.py b/tests/unit/test_composite.py index a9df414c..afb23e8c 100644 --- a/tests/unit/test_composite.py +++ b/tests/unit/test_composite.py @@ -596,27 +596,61 @@ def test_root(self): top.middle_composite.deep_node = Composite.create.SingleValue(plus_one) top.middle_function = Composite.create.SingleValue(plus_one) - self.assertIs( - top, - top.graph_root, - msg="The parent-most node should be its own graph_root." - ) - self.assertIs( - top, - top.middle_composite.graph_root, - msg="The parent-most node should be the graph_root." - ) - self.assertIs( - top, - top.middle_function.graph_root, - msg="The parent-most node should be the graph_root." - ) - self.assertIs( - top, - top.middle_composite.deep_node.graph_root, - msg="The parent-most node should be the graph_root, recursively accessible " - "from all depths." - ) + with self.subTest("test_graph_path"): + self.assertEqual( + top.label, + top.graph_path, + msg="The parent-most node should be its own path." + ) + self.assertEqual( + Composite._semantic_delimiter.join( + [top.label, top.middle_composite.label] + ), + top.middle_composite.graph_path, + msg="The path should go to the parent-most object." + ) + self.assertEqual( + Composite._semantic_delimiter.join( + [top.label, top.middle_function.label] + ), + top.middle_function.graph_path, + msg="The path should go to the parent-most object." + ) + self.assertEqual( + Composite._semantic_delimiter.join( + [ + top.label, + top.middle_composite.label, + top.middle_composite.deep_node.label + ] + ), + top.middle_composite.deep_node.graph_path, + msg="The path should go to the parent-most object, recursively from all " + "depths." + ) + + with self.subTest("test_graph_root"): + self.assertIs( + top, + top.graph_root, + msg="The parent-most node should be its own graph_root." + ) + self.assertIs( + top, + top.middle_composite.graph_root, + msg="The parent-most node should be the graph_root." + ) + self.assertIs( + top, + top.middle_function.graph_root, + msg="The parent-most node should be the graph_root." + ) + self.assertIs( + top, + top.middle_composite.deep_node.graph_root, + msg="The parent-most node should be the graph_root, recursively accessible " + "from all depths." + ) if __name__ == '__main__': diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py index 7372b811..10615838 100644 --- a/tests/unit/test_node.py +++ b/tests/unit/test_node.py @@ -346,6 +346,14 @@ def test_run_after_init(self): def test_root(self): n = ANode("n") + + self.assertEqual( + n.label, + n.graph_path, + msg="Lone nodes should just have their label as the path, as there is no " + "parent above." + ) + self.assertIs( n, n.graph_root, From b7e5d8f26550a70ed68f2d120a6c57acfdf6629c Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:51:14 -0800 Subject: [PATCH 16/17] Refactor: rename tests --- tests/unit/test_composite.py | 2 +- tests/unit/test_node.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_composite.py b/tests/unit/test_composite.py index afb23e8c..64185c74 100644 --- a/tests/unit/test_composite.py +++ b/tests/unit/test_composite.py @@ -590,7 +590,7 @@ def test_de_activate_strict_connections(self): msg="Activating should propagate to children" ) - def test_root(self): + def test_graph_info(self): top = AComposite("topmost") top.middle_composite = AComposite("middle_composite") top.middle_composite.deep_node = Composite.create.SingleValue(plus_one) diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py index 10615838..6b03ae86 100644 --- a/tests/unit/test_node.py +++ b/tests/unit/test_node.py @@ -344,7 +344,7 @@ def test_run_after_init(self): msg="With run_after_init, the node should run right away" ) - def test_root(self): + def test_graph_info(self): n = ANode("n") self.assertEqual( From c2ea143b361a05e5df7f96d00f6aa4f9f3b99a0d Mon Sep 17 00:00:00 2001 From: liamhuber Date: Thu, 18 Jan 2024 14:53:19 -0800 Subject: [PATCH 17/17] Remove the remote content again I really am not handling the local rebase + remote branch interaction nicely. This is even messier than just merging the upstream stuff to start with :facepalm: --- pyiron_workflow/node.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index 5ebee095..e30ebd0d 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -347,15 +347,6 @@ def graph_root(self) -> Node: """The parent-most node in this graph.""" return self if self.parent is None else self.parent.graph_root - @property - def semantic_path(self): - path = self.label - if self.parent is not None: - path = self.parent.semantic_path + self._semantic_delimiter + path - # else: - # path = self.semantic_root + self._semantic_delimiter + path - return path - @property def readiness_report(self) -> str: input_readiness = "\n".join(