diff --git a/CHANGELOG.md b/CHANGELOG.md index a71113f..c5a3af4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - Better subclassing support: Determine classes dynamically, so that methods like str() are aware when our types are subclassed. - Removed no-op check and extra list allocation during chop() + - Fixed unbalanced trees after bulk construction: srotate() now + re-balances the new tip after the promotees step, which could + prune a subtree and leave the node with a balance of +-2 - Maintainers: - Load issue4 test data via JSON to save on compilation and collection time. diff --git a/intervaltree/node.py b/intervaltree/node.py index 4993533..1b61b38 100644 --- a/intervaltree/node.py +++ b/intervaltree/node.py @@ -188,8 +188,11 @@ def srotate(self): # For now, this is the same as augmenting save.s_center, but that may # change. save.s_center.update(promotees) - save.refresh_balance() - return save + # Promoting intervals out of save[light] can leave it pruned, + # which un-balances save after the mechanical rotation above. + # Rebalance (a no-op when already balanced) instead of merely + # refreshing, so the returned tip respects the balance invariant. + return save.rotate() def drotate(self): # First rotation diff --git a/test/issues/issue135_test.py b/test/issues/issue135_test.py new file mode 100644 index 0000000..da44ef7 --- /dev/null +++ b/test/issues/issue135_test.py @@ -0,0 +1,68 @@ +""" +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, tree balance invariant after bulk construction +Submitted as issue #135 (specific intervals cause inbalanced nodes) + +Copyright 2013-2018 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 Interval, IntervalTree + + +# The exact set of intervals from the issue report that, when built +# into a tree at once, left the root node with balance +2 (its left +# subtree was entirely pruned away by the rotation promotees logic, +# but the node was only refresh_balance()'d, never re-balanced). +ISSUE135_INTERVALS = [ + (478019, 486075), + (478041, 483497), + (478215, 486619), + (478416, 488227), + (479107, 483366), + (479112, 488300), + (479867, 489663), + (481266, 490406), + (482870, 485066), + (483050, 487543), + (484692, 485957), + (488408, 489859), + (490057, 492197), + (491277, 493556), +] + + +def test_bulk_construction_is_balanced(): + # regression: building this set at once raised + # AssertionError: Error: Rotation should have happened, but didn't! + t = IntervalTree.from_tuples(ISSUE135_INTERVALS) + t.verify() + # and no interval was lost in the process + assert sorted(t) == sorted(Interval(*iv) for iv in ISSUE135_INTERVALS) + + +def test_sequential_add_is_balanced(): + # the same intervals added one at a time must also stay balanced + t = IntervalTree() + for iv in ISSUE135_INTERVALS: + t.addi(iv[0], iv[1]) + t.verify() + assert sorted(t) == sorted(Interval(*iv) for iv in ISSUE135_INTERVALS) + + +if __name__ == "__main__": + test_bulk_construction_is_balanced() + test_sequential_add_is_balanced()