Skip to content

Commit bbca53d

Browse files
committed
Document the fact OrderedSet.remove is linear
This has potential to cause quadratic blowup. Add a test to check this (in case we ever fix it).
1 parent b011f13 commit bbca53d

File tree

2 files changed

+75
-0
lines changed

2 files changed

+75
-0
lines changed

src/techcable/orderedset/_orderedset.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ class OrderedSet(MutableSet[T], Sequence[T]):
7070
because [`OrderedSet.append`] ignores duplicate elements
7171
and returns `bool` instead of `None`.
7272
73+
The [`OrderedSet.remove`] method takes linear time to remove an element,
74+
not constant time like `set.remove` does.
75+
Calling it in a loop will trigger quadratic blow up just like use of [`list.remove`] would.
76+
To avoid this, bulk-remove elements using the set subtraction operator (`-=`).
77+
7378
### Thread Safety
7479
This type is *NOT* safe to mutate from multiple threads.
7580
@@ -167,6 +172,11 @@ def remove(self, value: T, /) -> None:
167172
"""
168173
Remove an element from the set, throwing a KeyError if not present.
169174
175+
This method preserves the original order of the set.
176+
However, it takes linear time like [`list.remove`],
177+
instead of the constant time that [`set.remove`] takes.
178+
Invoking it repeatedly may cause quadratic blowup, just like `list.remove` would.
179+
170180
See [`OrderedSet.discard`] for a variant that does nothing if the item is not present.
171181
"""
172182
# set.remove will raise a KeyError for us
@@ -178,6 +188,11 @@ def discard(self, value: T, /) -> None:
178188
"""
179189
Remove an element from the set if it exists.
180190
191+
This method preserves the original order of the set.
192+
However, it takes linear time (`O(n)`) instead of the constant time.
193+
Invoking it repeatedly may cause quadratic blowup.
194+
See [`OrderedSet.remove`] for more details on this.
195+
181196
Unlike [`OrderedSet.remove`], this method does not raise
182197
an exception if this element is missing.
183198
"""

tests/test_remove_complexity.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
"""
2+
Ensures the [`OrderedSet.remove`] function runs in constant time instead of linear time.
3+
"""
4+
5+
import threading
6+
from typing import ClassVar
7+
8+
import pytest
9+
10+
from techcable.orderedset import OrderedSet
11+
12+
13+
class UnexpectedComplexityError(AssertionError):
14+
pass
15+
16+
17+
class EqualityCounter(threading.local):
18+
comparisons: int
19+
hashes: int = 0
20+
21+
def __init__(self) -> None:
22+
super().__init__()
23+
self.reset()
24+
25+
def reset(self) -> None:
26+
self.comparisons = 0
27+
self.hashes = 0
28+
29+
30+
class EqualityCounted:
31+
"""A type that counts the number of times it has been compared against."""
32+
33+
value: int
34+
COUNTER: ClassVar = EqualityCounter()
35+
36+
def __init__(self, value: int, /):
37+
self.value = value
38+
39+
def __eq__(self, other: object) -> bool:
40+
if isinstance(other, EqualityCounted):
41+
self.COUNTER.comparisons += 1
42+
return self.value == other.value
43+
else:
44+
return NotImplemented
45+
46+
def __hash__(self) -> int:
47+
self.COUNTER.hashes += 1
48+
return hash(self.value)
49+
50+
def __repr__(self) -> str:
51+
return f"EqualityCounter({self.value})"
52+
53+
54+
@pytest.mark.xfail(raises=UnexpectedComplexityError, reason="Remove is still linear")
55+
def test_const_remove():
56+
oset = OrderedSet(EqualityCounted(i) for i in range(10_000))
57+
EqualityCounted.COUNTER.reset()
58+
oset.remove(EqualityCounted(5000))
59+
if (comparison_count := EqualityCounted.COUNTER.comparisons) > 10:
60+
raise UnexpectedComplexityError(f"oset.remove: {comparison_count}")

0 commit comments

Comments
 (0)