Merge pull request #42 from escalonn/issue41

Resolve issue #41

Author: chaimleib

intervaltree/node.py

@@ -372,10 +372,7 @@ def get_new_s_center():
                     if iv.contains_point(new_x_center): yield iv
 
             # Create a new node with the largest x_center possible.
-            child = Node.from_intervals(get_new_s_center())
-            #     [iv for iv in self.s_center if iv.contains_point(child_x_center)]
-            # )
-            child.x_center = new_x_center
+            child = Node(new_x_center, get_new_s_center())
             self.s_center -= child.s_center
 
             #print('Pop hit! Returning child   = {}'.format(

test/data/issue41_orig.py

@@ -0,0 +1,92 @@
+"""
+Manually create the original tree that revealed the bug. I have to
+do this, since with later code changes, the original sequence of
+insertions and removals might not recreate that tree exactly, with
+the same internal structure.
+
+Node<961, depth=2, balance=0>
+ Interval(961, 986, 1)
+<:  Node<871, depth=1, balance=0>
+     Interval(860, 917, 1)
+     Interval(860, 917, 2)
+     Interval(860, 917, 3)
+     Interval(860, 917, 4)
+     Interval(871, 917, 1)
+     Interval(871, 917, 2)
+     Interval(871, 917, 3)
+>:  Node<1047, depth=1, balance=0>
+     Interval(1047, 1064, 1)
+     Interval(1047, 1064, 2)
+"""
+from intervaltree import IntervalTree, Interval
+from intervaltree.node import Node
+
+data = [
+    (860, 917, 1),   #0
+    (860, 917, 2),   #1
+    (860, 917, 3),   #2
+    (860, 917, 4),   #3
+    (871, 917, 1),   #4
+    (871, 917, 2),   #5
+    (871, 917, 3),   #6
+    (961, 986, 1),   #7
+    (1047, 1064, 1), #8
+    (1047, 1064, 2), #9
+]
+
+def tree():
+    t = IntervalTree.from_tuples(data)
+    # Node<961, depth=2, balance=0>
+    #  Interval(961, 986, 1)
+    root = Node()
+    root.x_center = 961
+    root.s_center = set([Interval(*data[7])])
+    root.depth = 2
+    root.balance = 0
+
+    # <:  Node<871, depth=1, balance=0>
+    #      Interval(860, 917, 1)
+    #      Interval(860, 917, 2)
+    #      Interval(860, 917, 3)
+    #      Interval(860, 917, 4)
+    #      Interval(871, 917, 1)
+    #      Interval(871, 917, 2)
+    #      Interval(871, 917, 3)
+    n = root.left_node = Node()
+    n.x_center = 871
+    n.s_center = set(Interval(*tup) for tup in data[:7])
+    n.depth = 1
+    n.balance = 0
+
+    # >:  Node<1047, depth=1, balance=0>
+    #      Interval(1047, 1064, 1)
+    #      Interval(1047, 1064, 2)
+    n = root.right_node = Node()
+    n.x_center = 1047
+    n.s_center = set(Interval(*tup) for tup in data[8:])
+    n.depth = 1
+    n.balance = 0
+
+    structure = root.print_structure(tostring=True)
+    # root.print_structure()
+    assert structure == """\
+Node<961, depth=2, balance=0>
+ Interval(961, 986, 1)
+<:  Node<871, depth=1, balance=0>
+     Interval(860, 917, 1)
+     Interval(860, 917, 2)
+     Interval(860, 917, 3)
+     Interval(860, 917, 4)
+     Interval(871, 917, 1)
+     Interval(871, 917, 2)
+     Interval(871, 917, 3)
+>:  Node<1047, depth=1, balance=0>
+     Interval(1047, 1064, 1)
+     Interval(1047, 1064, 2)
+"""
+    t.top_node = root
+    t.verify()
+    return t
+
+if __name__ == "__main__":
+    tree().print_structure()

test/issue41_test.py

@@ -0,0 +1,92 @@
+"""
+intervaltree: A mutable, self-balancing interval tree for Python 2 and 3.
+Queries may be by point, by range overlap, or by range envelopment.
+
+Test module: IntervalTree, removal of intervals
+Submitted as issue #41 (Interval removal breaks this tree) by escalonn
+
+Copyright 2013-2015 Chaim-Leib Halbert
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+from __future__ import absolute_import
+from intervaltree import IntervalTree
+from test.intervaltrees import trees
+import pytest
+
+
+def test_original_sequence():
+    t = IntervalTree()
+    t.addi(860, 917, 1)
+    t.addi(860, 917, 2)
+    t.addi(860, 917, 3)
+    t.addi(860, 917, 4)
+    t.addi(871, 917, 1)
+    t.addi(871, 917, 2)
+    t.addi(871, 917, 3)     # Value inserted here
+    t.addi(961, 986, 1)
+    t.addi(1047, 1064, 1)
+    t.addi(1047, 1064, 2)
+    t.removei(961, 986, 1)
+    t.removei(871, 917, 3)  # Deleted here
+
+
+def test_debug_sequence():
+    t = IntervalTree()
+    t.addi(860, 917, 1)
+    t.verify()
+    t.addi(860, 917, 2)
+    t.verify()
+    t.addi(860, 917, 3)
+    t.verify()
+    t.addi(860, 917, 4)
+    t.verify()
+    t.addi(871, 917, 1)
+    t.verify()
+    t.addi(871, 917, 2)
+    t.verify()
+    t.addi(871, 917, 3)     # Value inserted here
+    t.verify()
+    t.addi(961, 986, 1)
+    t.verify()
+    t.addi(1047, 1064, 1)
+    t.verify()
+    t.addi(1047, 1064, 2)
+    t.verify()
+    t.removei(961, 986, 1)
+    t.verify()
+    t.removei(871, 917, 3)  # Deleted here
+    t.verify()
+
+
+def test_orig_structure():
+    """
+    Reconstruct the original tree just before the removals, then
+    perform the removals.
+    """
+    t = trees['issue41_orig']()
+    # t.print_structure()
+    t.verify()
+
+    t.removei(961, 986, 1)
+    # t.print_structure()
+    t.verify()
+
+    t.removei(871, 917, 3)
+    # t.print_structure()
+    t.verify()
+
+
+if __name__ == "__main__":
+    # pytest.main([__file__, '-v'])
+    test_orig_structure()