Merge pull request #270 from scipp/fix_connecting_draw_tool_to_node

Fix connecting draw tool to node

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.add_child(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()
 
@@ -120,17 +132,27 @@ def request_data(self) -> Any:
             self._data = self.func(*args, **kwargs)
         return self._data
 
-    def add_child(self, child: Node):
+    def add_parents(self, *parents: Node):
+        """
+        Add one or more parents to the node.
+        """
+        for parent in parents:
+            _no_replace_append(self.parents, parent, 'parent')
+            _no_replace_append(parent.children, self, 'child')
+
+    def add_kwparents(self, **parents: Node):
         """
-        Add a child to the node.
+        Add one or more keyword parents to the node.
         """
-        self.children.append(child)
+        for key, parent in parents.items():
+            self.kwparents[key] = parent
+            _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):

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)
@@ -87,7 +87,8 @@ 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)
+            self._destination.add_parents(output_node)
+            self._destination.notify_children(artist)
 
     def update_node(self, artist):
         n = self._draw_nodes[artist.nodeid]

tests/core/node_test.py

@@ -259,3 +259,90 @@ 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
+
+
+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)