Skip to content

Commit 0190809

Browse files
committed
Fix P1 bug: Validate index when erasing last element
PROBLEM: When tree has single element, erase() bypassed index validation and silently deleted the element regardless of which index was requested. Example: tree with index 5, calling erase(999) would succeed and delete index 5 without error. ROOT CAUSE: The workaround for C++ library bug ("#roots is not 1") immediately recreated an empty tree without validating that the requested index actually existed in the tree. FIX: Call underlying _tree.erase(idx) first to validate the index: - If "Given index is not found" -> re-raise (invalid index) - If "#roots is not 1" -> recreate empty tree (valid index, library bug) - Otherwise -> re-raise (other error) TESTS ADDED: - test_erase_non_existent_index: Now expects RuntimeError (was lenient) - test_erase_non_existent_index_single_element: P1 bug regression test - test_erase_valid_index_single_element: Verify valid erase still works Result: 950 tests passed (6 new tests added)
1 parent f04ffbb commit 0190809

File tree

2 files changed

+67
-12
lines changed

2 files changed

+67
-12
lines changed

src/python_prtree/__init__.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,24 @@ def erase(self, idx):
5858

5959
# Handle erasing the last element (library limitation workaround)
6060
if self.n == 1:
61-
# Recreate an empty tree (workaround for C++ limitation)
62-
self._tree = self.Klass()
63-
return
61+
# Call underlying erase to validate index, then handle the library bug
62+
try:
63+
self._tree.erase(idx)
64+
# If we get here, erase succeeded (shouldn't happen with n==1)
65+
return
66+
except RuntimeError as e:
67+
error_msg = str(e)
68+
if "Given index is not found" in error_msg:
69+
# Index doesn't exist - re-raise the error
70+
raise
71+
elif "#roots is not 1" in error_msg:
72+
# This is the library bug we're working around
73+
# Index was valid, so recreate empty tree
74+
self._tree = self.Klass()
75+
return
76+
else:
77+
# Some other RuntimeError - re-raise it
78+
raise
6479

6580
self._tree.erase(idx)
6681

tests/unit/test_erase.py

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def test_erase_from_empty_tree(self, PRTree, dim):
5656

5757
@pytest.mark.parametrize("PRTree, dim", [(PRTree2D, 2), (PRTree3D, 3), (PRTree4D, 4)])
5858
def test_erase_non_existent_index(self, PRTree, dim):
59-
"""存在しないインデックスの削除の動作確認."""
59+
"""存在しないインデックスの削除がエラーになることを確認."""
6060
idx = np.array([1, 2])
6161
boxes = np.zeros((2, 2 * dim))
6262
for i in range(2):
@@ -66,15 +66,55 @@ def test_erase_non_existent_index(self, PRTree, dim):
6666

6767
tree = PRTree(idx, boxes)
6868

69-
# Try to erase non-existent index
70-
# Implementation may raise error or silently fail
71-
try:
69+
# Try to erase non-existent index - should raise error
70+
with pytest.raises(RuntimeError, match="Given index is not found"):
7271
tree.erase(999)
73-
# If no error, size should remain same
74-
assert tree.size() == 2
75-
except (ValueError, RuntimeError, KeyError):
76-
# Error is also acceptable
77-
pass
72+
73+
# Tree should be unchanged
74+
assert tree.size() == 2
75+
76+
@pytest.mark.parametrize("PRTree, dim", [(PRTree2D, 2), (PRTree3D, 3), (PRTree4D, 4)])
77+
def test_erase_non_existent_index_single_element(self, PRTree, dim):
78+
"""単一要素のツリーで存在しないインデックスの削除がエラーになることを確認 (P1 validation bug)."""
79+
idx = np.array([5])
80+
boxes = np.zeros((1, 2 * dim))
81+
for d in range(dim):
82+
boxes[0, d] = 0.0
83+
boxes[0, d + dim] = 1.0
84+
85+
tree = PRTree(idx, boxes)
86+
assert tree.size() == 1
87+
88+
# Try to erase non-existent index 999 - should raise error
89+
# This is the P1 bug: previously silently deleted the real element
90+
with pytest.raises(RuntimeError, match="Given index is not found"):
91+
tree.erase(999)
92+
93+
# Tree should still contain the element
94+
assert tree.size() == 1
95+
96+
# Verify the correct element is still there
97+
query_box = boxes[0]
98+
result = tree.query(query_box)
99+
assert 5 in result
100+
101+
@pytest.mark.parametrize("PRTree, dim", [(PRTree2D, 2), (PRTree3D, 3), (PRTree4D, 4)])
102+
def test_erase_valid_index_single_element(self, PRTree, dim):
103+
"""単一要素のツリーで有効なインデックスの削除が機能することを確認."""
104+
idx = np.array([5])
105+
boxes = np.zeros((1, 2 * dim))
106+
for d in range(dim):
107+
boxes[0, d] = 0.0
108+
boxes[0, d + dim] = 1.0
109+
110+
tree = PRTree(idx, boxes)
111+
assert tree.size() == 1
112+
113+
# Erase the valid index 5 - should succeed
114+
tree.erase(5)
115+
116+
# Tree should now be empty
117+
assert tree.size() == 0
78118

79119

80120
class TestConsistencyErase:

0 commit comments

Comments
 (0)