Merge pull request #2 from leonardbinet/remove_node_hierarchy

Remove node hierarchy

Author: leonardbinet

lighttree/__init__.py

@@ -1,5 +1,4 @@
-from .node import Node
-from .tree import Tree
+from .tree import Tree, Node
 from .interactive import TreeBasedObj
 
 __all__ = ["Tree", "Node", "TreeBasedObj"]

lighttree/node.py

@@ -1,14 +1,16 @@
 #!/usr/bin/env python
+# -*- coding: utf-8 -*-
 
 from __future__ import unicode_literals
+import copy
 from future.utils import python_2_unicode_compatible, string_types
 
 import uuid
 
 
 @python_2_unicode_compatible
 class Node(object):
-    def __init__(self, identifier=None, auto_uuid=False, _children=None):
+    def __init__(self, identifier=None, auto_uuid=False):
         """
         :param identifier: node identifier, must be unique per tree
         """
@@ -22,18 +24,15 @@ def __init__(self, identifier=None, auto_uuid=False, _children=None):
                 raise ValueError("Required identifier")
             identifier = uuid.uuid4()
         self.identifier = identifier
-        # children type is not checked here, it is at insertion in tree
-        # only allowed types should be Node, or Tree, but cannot ensure whether it's a Tree since it would cause
-        # a recursive import error
-        if _children is not None and not isinstance(_children, (list, tuple)):
-            raise ValueError("Invalid children declaration.")
-        self._children = _children
 
     def line_repr(self, depth, **kwargs):
         """Control how node is displayed in tree representation.
         """
         return self.identifier
 
+    def clone(self, deep=False):
+        return copy.deepcopy(self) if deep else copy.copy(self)
+
     def serialize(self, *args, **kwargs):
         return {"identifier": self.identifier}
 
@@ -47,6 +46,11 @@ def deserialize(cls, d, *args, **kwargs):
     def _deserialize(cls, d, *args, **kwargs):
         return cls(d.get("identifier"))
 
+    def __eq__(self, other):
+        if not isinstance(other, self.__class__):
+            return False
+        return self.identifier == other.identifier
+
     def __str__(self):
         return "%s, id=%s" % (self.__class__.__name__, self.identifier)
 

lighttree/tree.py

@@ -5,10 +5,9 @@
 from future.utils import python_2_unicode_compatible, iteritems
 
 from collections import defaultdict
-from copy import deepcopy
 from operator import attrgetter
 
-from .node import Node
+from lighttree.node import Node
 from .utils import STYLES
 from .exceptions import MultipleRootError, NotFoundNodeError, DuplicatedNodeError
 
@@ -72,7 +71,7 @@ def _validate_node_insertion(self, node):
             )
 
     def _validate_tree_insertion(self, tree):
-        if not isinstance(tree, self.__class__):
+        if not isinstance(tree, Tree):
             raise ValueError(
                 "Tree must be instance of <%s>, got <%s>"
                 % (self.__class__.__name__, type(tree))
@@ -114,11 +113,8 @@ def clone(self, with_tree=True, deep=False, new_root=None):
 
         for nid in self.expand_tree(nid=new_root):
             node = self.get(nid)
-            if deep:
-                node = deepcopy(node)
             pid = None if nid == self.root or nid == new_root else self.parent(nid)
-            # with_children only makes sense when using "node hierarchy" syntax
-            new_tree.insert_node(node, parent_id=pid, with_children=False)
+            new_tree.insert_node(node, parent_id=pid, deep=deep)
         return new_tree
 
     def parent(self, nid, id_only=True):
@@ -210,39 +206,36 @@ def insert(
             '"item" parameter must either be a Node, or a Tree, got <%s>.' % type(item)
         )
 
-    def insert_node(
-        self, node, parent_id=None, child_id=None, deep=False, with_children=True
-    ):
+    def insert_node(self, node, parent_id=None, child_id=None, deep=False):
+        """Make a copy of inserted node, and insert it.
+
+        Note: when using "Node hierarchy" syntax, _children attribute of copied node are reset so that insertion occurs
+        once only.
+        """
         self._validate_node_insertion(node)
-        node = deepcopy(node) if deep else node
+        node = node.clone(deep=deep)
         if parent_id is not None and child_id is not None:
             raise ValueError('Can declare at most "parent_id" or "child_id"')
         if child_id is not None:
             self._insert_node_above(node, child_id=child_id)
             return self
-        self._insert_node_below(node, parent_id=parent_id, with_children=with_children)
+        self._insert_node_below(node, parent_id=parent_id)
         return self
 
-    def _insert_node_below(self, node, parent_id, with_children=True):
+    def _insert_node_below(self, node, parent_id):
         # insertion at root
         if parent_id is None:
             if not self.is_empty():
                 raise MultipleRootError("A tree takes one root merely.")
             self.root = node.identifier
             self._nodes_map[node.identifier] = node
-            if with_children and hasattr(node, "_children"):
-                for child in node._children or []:
-                    self.insert(child, parent_id=node.identifier)
             return
 
         self._ensure_present(parent_id)
         node_id = node.identifier
         self._nodes_map[node_id] = node
         self._nodes_parent[node_id] = parent_id
         self._nodes_children[parent_id].add(node_id)
-        if with_children and hasattr(node, "_children"):
-            for child in node._children or []:
-                self.insert(child, parent_id=node.identifier)
 
     def _insert_node_above(self, node, child_id):
         self._ensure_present(child_id)
@@ -280,10 +273,8 @@ def _insert_tree_below(self, new_tree, parent_id, deep):
         for new_nid in new_tree.expand_tree():
             node = new_tree.get(new_nid)
             pid = parent_id if new_nid == new_tree.root else new_tree.parent(new_nid)
-            # with_children only makes sense when using "node hierarchy" syntax
-            self.insert_node(
-                deepcopy(node) if deep else node, parent_id=pid, with_children=False
-            )
+            # node copy is handled in insert_node method
+            self.insert_node(node, parent_id=pid, deep=deep)
         return self
 
     def _insert_tree_above(self, new_tree, child_id, child_id_below, deep):

setup.py

@@ -3,7 +3,7 @@
 
 from setuptools import setup
 
-__version__ = "0.0.3"
+__version__ = "0.0.5"
 
 
 setup(

tests/test_interactive.py

@@ -188,8 +188,7 @@ class InteractiveTree(TreeBasedObj):
         └── a2
         """
         self.assertTrue(hasattr(a, "a1"))
-        # check that initial tree, and child tree reference the same nodes
-        self.assertIs(a._tree.get("a1"), obj._tree.get("a1"))
+        self.assertEqual(a._tree.get("a1"), obj._tree.get("a1"))
 
         # test representations
         self.assertEqual(

tests/test_tree.py

@@ -70,7 +70,7 @@ def test_insert_node_below(self):
         node_a = Node("a")
         t.insert_node(node_a, parent_id="root_id")
         self.assertSetEqual(set(t._nodes_map.keys()), {"root_id", "a"})
-        self.assertIs(t._nodes_map["a"], node_a)
+        self.assertEqual(t._nodes_map["a"], node_a)
         self.assertEqual(t._nodes_parent["root_id"], None)
         self.assertEqual(t._nodes_parent["a"], "root_id")
         self.assertSetEqual(t._nodes_children["a"], set())
@@ -245,7 +245,7 @@ def test_clone_with_tree(self):
         self.assertIs(t.mutable_object, t_shallow_clone.mutable_object)
         # nodes are shallow copies
         for nid, node in iteritems(t._nodes_map):
-            self.assertIs(t_shallow_clone._nodes_map[nid], node)
+            self.assertEqual(t_shallow_clone._nodes_map[nid], node)
         tree_sanity_check(t)
         tree_sanity_check(t_shallow_clone)
 
@@ -637,7 +637,7 @@ def test_insert_tree_below(self):
         )
         self.assertTrue(all(nid in t for nid in ("c", "c1", "c2", "c12")))
         # by default pasted new tree is a shallow copy
-        self.assertIs(t.get("c"), t_to_paste.get("c"))
+        self.assertEqual(t.get("c"), t_to_paste.get("c"))
 
         # cannot repaste tree, because then there would be node duplicates
         with self.assertRaises(DuplicatedNodeError):
@@ -779,8 +779,7 @@ def test_merge(self):
         # new tree root is not conserved
         self.assertTrue("c" not in t)
         self.assertTrue(all(nid in t for nid in ("c1", "c2", "c12")))
-        # by default merged new tree is a shallow copy
-        self.assertIs(t.get("c1"), t_to_merge.get("c1"))
+        self.assertEqual(t.get("c1"), t_to_merge.get("c1"))
 
         # cannot remerge tree, because then there would be node duplicates
         with self.assertRaises(DuplicatedNodeError):
@@ -888,58 +887,5 @@ def test_drop_subtree(self):
             """a1
 ├── a11
 └── a12
-""",
-        )
-
-    def test_node_hierarchy_deserialization(self):
-        node_hierarchy = Node(
-            identifier="root",
-            _children=[
-                Node(
-                    identifier="a",
-                    _children=[Node(identifier="a1"), Node(identifier="a2")],
-                ),
-                Node(identifier="b", _children=[Node(identifier="b1")]),
-            ],
-        )
-        t = Tree()
-        t.insert(node_hierarchy)
-        self.assertEqual(
-            t.show(),
-            """root
-├── a
-│   ├── a1
-│   └── a2
-└── b
-    └── b1
-""",
-        )
-
-    def test_node_hierarchy_with_tree_deserialization(self):
-        node_hierarchy = Node(
-            identifier="root",
-            _children=[
-                Node(
-                    identifier="a",
-                    _children=[Node(identifier="a1"), Node(identifier="a2")],
-                ),
-                Node(identifier="b", _children=[Node(identifier="b1")]),
-                get_sample_tree_2(),
-            ],
-        )
-        t = Tree()
-        t.insert(node_hierarchy)
-        self.assertEqual(
-            t.show(),
-            """root
-├── a
-│   ├── a1
-│   └── a2
-├── b
-│   └── b1
-└── c
-    ├── c1
-    │   └── c12
-    └── c2
 """,
         )

tests/testing_utils.py

@@ -102,6 +102,9 @@ def __init__(self, identifier, key):
         self.key = key
         super(CustomNode, self).__init__(identifier=identifier)
 
+    def clone(self, deep=False):
+        return self.__class__(identifier=self.identifier, key=self.key,)
+
     def serialize(self, *args, **kwargs):
         with_key = kwargs.pop("with_key", None)
         d = super(CustomNode, self).serialize()