From 719e63c97880e2b319b884dd635445609964761b Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Mon, 2 Oct 2023 11:51:14 +0200 Subject: [PATCH 1/4] fix connecting draw tool to node --- src/plopp/widgets/drawing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plopp/widgets/drawing.py b/src/plopp/widgets/drawing.py index 498beaa5..d466133a 100644 --- a/src/plopp/widgets/drawing.py +++ b/src/plopp/widgets/drawing.py @@ -59,13 +59,13 @@ def __init__( super().__init__(callback=self.start_stop, value=value, **kwargs) self._figure = figure - self._destination_is_fig = is_figure(self._figure) self._input_node = input_node self._draw_nodes = {} self._output_nodes = {} self._func = func self._tool = tool(ax=self._figure.ax, autostart=False) self._destination = destination + self._destination_is_fig = is_figure(self._destination) self._get_artist_info = get_artist_info self._tool.on_create(self.make_node) self._tool.on_change(self.update_node) @@ -88,6 +88,8 @@ def make_node(self, artist): ) elif isinstance(self._destination, Node): self._destination.parents.append(output_node) + output_node.add_child(self._destination) + self._destination.notify_children(artist) def update_node(self, artist): n = self._draw_nodes[artist.nodeid] From bd29e82afd4bc5d7b4101dffa43ea4f3b9bbd7b4 Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Mon, 2 Oct 2023 19:05:51 +0200 Subject: [PATCH 2/4] replace add_child method with add_parent --- src/plopp/core/node.py | 16 ++++++++++++---- src/plopp/widgets/drawing.py | 3 +-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/plopp/core/node.py b/src/plopp/core/node.py index 515617bf..9c7ae836 100644 --- a/src/plopp/core/node.py +++ b/src/plopp/core/node.py @@ -44,7 +44,7 @@ def __init__(self, func: Any, *parents, **kwparents): key: p if isinstance(p, Node) else Node(p) for key, p in kwparents.items() } for parent in chain(self.parents, self.kwparents.values()): - parent.add_child(self) + parent.children.append(self) self._data = None if func_is_callable: @@ -120,11 +120,19 @@ def request_data(self) -> Any: self._data = self.func(*args, **kwargs) return self._data - def add_child(self, child: Node): + def add_parent(self, parent: Node): """ - Add a child to the node. + Add a parent to the node. """ - self.children.append(child) + self.parents.append(parent) + parent.children.append(self) + + def add_kwparent(self, key: str, parent: Node): + """ + Add a keyword parent to the node. + """ + self.kwparents[key] = parent + parent.children.append(self) def add_view(self, view: View): """ diff --git a/src/plopp/widgets/drawing.py b/src/plopp/widgets/drawing.py index d466133a..ddf100ba 100644 --- a/src/plopp/widgets/drawing.py +++ b/src/plopp/widgets/drawing.py @@ -87,8 +87,7 @@ def make_node(self, artist): artist.color if hasattr(artist, 'color') else artist.edgecolor ) elif isinstance(self._destination, Node): - self._destination.parents.append(output_node) - output_node.add_child(self._destination) + self._destination.add_parent(output_node) self._destination.notify_children(artist) def update_node(self, artist): From 598ff9de311bc019141945ba6ddbcb0e6d99c82d Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Mon, 2 Oct 2023 19:19:29 +0200 Subject: [PATCH 3/4] add tests and allow adding more than one parent at once --- src/plopp/core/node.py | 18 +++++++------ src/plopp/widgets/drawing.py | 2 +- tests/core/node_test.py | 50 ++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/plopp/core/node.py b/src/plopp/core/node.py index 9c7ae836..67b4c31d 100644 --- a/src/plopp/core/node.py +++ b/src/plopp/core/node.py @@ -120,19 +120,21 @@ def request_data(self) -> Any: self._data = self.func(*args, **kwargs) return self._data - def add_parent(self, parent: Node): + def add_parents(self, *parents: Node): """ - Add a parent to the node. + Add one or more parents to the node. """ - self.parents.append(parent) - parent.children.append(self) + for parent in parents: + self.parents.append(parent) + parent.children.append(self) - def add_kwparent(self, key: str, parent: Node): + def add_kwparents(self, **parents: Node): """ - Add a keyword parent to the node. + Add one or more keyword parents to the node. """ - self.kwparents[key] = parent - parent.children.append(self) + for key, parent in parents.items(): + self.kwparents[key] = parent + parent.children.append(self) def add_view(self, view: View): """ diff --git a/src/plopp/widgets/drawing.py b/src/plopp/widgets/drawing.py index ddf100ba..4752bd1a 100644 --- a/src/plopp/widgets/drawing.py +++ b/src/plopp/widgets/drawing.py @@ -87,7 +87,7 @@ def make_node(self, artist): artist.color if hasattr(artist, 'color') else artist.edgecolor ) elif isinstance(self._destination, Node): - self._destination.add_parent(output_node) + self._destination.add_parents(output_node) self._destination.notify_children(artist) def update_node(self, artist): diff --git a/tests/core/node_test.py b/tests/core/node_test.py index d1d2ab73..cdc79234 100644 --- a/tests/core/node_test.py +++ b/tests/core/node_test.py @@ -259,3 +259,53 @@ def mult(x, y): b = Node(4.0) c = mult(x=a, y=b) assert c() == 24.0 + + +def test_add_parent(): + a = Node(lambda: 5) + b = Node(lambda x: x - 2) + assert not a.children + assert not b.parents + assert not b.kwparents + b.add_parents(a) + assert a in b.parents + assert b in a.children + + +def test_add_multiple_parents(): + a = Node(lambda: 5.0) + b = Node(lambda: 12.0) + c = Node(lambda: -3.0) + d = Node(lambda x, y, z: x * y * z) + d.add_parents(a, b, c) + assert a in d.parents + assert b in d.parents + assert c in d.parents + assert d in a.children + assert d in b.children + assert d in c.children + + +def test_add_kwparents(): + a = Node(lambda: 5) + b = Node(lambda time: time * 101.0) + assert not a.children + assert not b.parents + assert not b.kwparents + b.add_kwparents(time=a) + assert a is b.kwparents['time'] + assert b in a.children + + +def test_add_multiple_kwparents(): + a = Node(lambda: 5.0) + b = Node(lambda: 12.0) + c = Node(lambda: -3.0) + d = Node(lambda x, y, z: x * y * z) + d.add_kwparents(y=a, z=b, x=c) + assert a is d.kwparents['y'] + assert b is d.kwparents['z'] + assert c is d.kwparents['x'] + assert d in a.children + assert d in b.children + assert d in c.children From 880b2d4e04886dc94c44888193825ccccb002e95 Mon Sep 17 00:00:00 2001 From: Neil Vaytet Date: Tue, 3 Oct 2023 11:21:33 +0200 Subject: [PATCH 4/4] prevent adding the same child, parent, view twice in graph --- src/plopp/core/node.py | 26 +++++++++++++++++++------- tests/core/node_test.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/plopp/core/node.py b/src/plopp/core/node.py index 67b4c31d..656e4d75 100644 --- a/src/plopp/core/node.py +++ b/src/plopp/core/node.py @@ -5,11 +5,21 @@ import uuid from itertools import chain -from typing import Any, Union +from typing import Any, List, Union from .view import View +def _no_replace_append(container: List[Node], item: Node, kind: str): + """ + Append ``item`` to ``container`` if it is not already in it. + """ + if item in container: + tpe = 'View' if kind == 'view' else 'Node' + raise ValueError(f"{tpe} {item} is already a {kind} in {container}.") + container.append(item) + + class Node: """ A node that can have parent and children nodes, to create a graph. @@ -43,8 +53,6 @@ def __init__(self, func: Any, *parents, **kwparents): self.kwparents = { key: p if isinstance(p, Node) else Node(p) for key, p in kwparents.items() } - for parent in chain(self.parents, self.kwparents.values()): - parent.children.append(self) self._data = None if func_is_callable: @@ -61,6 +69,10 @@ def __init__(self, func: Any, *parents, **kwparents): val_str = f'={repr(func)}' if isinstance(func, (int, float, str)) else "" self.name = f'Input <{type(func).__name__}{val_str}>' + # Attempt to set children after setting name in case error message is needed + for parent in chain(self.parents, self.kwparents.values()): + _no_replace_append(parent.children, self, 'child') + def __call__(self): return self.request_data() @@ -125,8 +137,8 @@ def add_parents(self, *parents: Node): Add one or more parents to the node. """ for parent in parents: - self.parents.append(parent) - parent.children.append(self) + _no_replace_append(self.parents, parent, 'parent') + _no_replace_append(parent.children, self, 'child') def add_kwparents(self, **parents: Node): """ @@ -134,13 +146,13 @@ def add_kwparents(self, **parents: Node): """ for key, parent in parents.items(): self.kwparents[key] = parent - parent.children.append(self) + _no_replace_append(parent.children, self, 'child') def add_view(self, view: View): """ Add a view to the node. """ - self.views.append(view) + _no_replace_append(self.views, view, 'view') view.graph_nodes[self.id] = self def notify_children(self, message: Any): diff --git a/tests/core/node_test.py b/tests/core/node_test.py index cdc79234..87b149c0 100644 --- a/tests/core/node_test.py +++ b/tests/core/node_test.py @@ -309,3 +309,40 @@ def test_add_multiple_kwparents(): assert d in a.children assert d in b.children assert d in c.children + + +def test_adding_same_child_twice_raises(): + a = Node(lambda: 5) + with pytest.raises(ValueError, match="Node .* is already a child in"): + Node(lambda x, y: x * y - 2, a, a) + with pytest.raises(ValueError, match="Node .* is already a child in"): + Node(lambda x, y: x * y - 2, x=a, y=a) + + +def test_adding_same_parent_twice_raises(): + a = Node(lambda: 5) + b = Node(lambda x, y: x * y - 2) + b.add_parents(a) + with pytest.raises(ValueError, match="Node .* is already a parent in"): + b.add_parents(a) + + +def test_adding_same_parent_twice_at_once_raises(): + a = Node(lambda: 5) + b = Node(lambda x, y: x * y - 2) + with pytest.raises(ValueError, match="Node .* is already a parent in"): + b.add_parents(a, a) + + +def test_adding_same_kwparent_twice_raises(): + a = Node(lambda: 5) + b = Node(lambda x, y: x * y - 2) + with pytest.raises(ValueError, match="Node .* is already a child in"): + b.add_kwparents(x=a, y=a) + + +def test_adding_same_view_twice_raises(): + a = Node(lambda: 15.0) + av = SimpleView(a) + with pytest.raises(ValueError, match="View .* is already a view in"): + a.add_view(av)