Skip to content

Fix PDeque.remove() and PDeque.reverse() dropping maxlen - #331

Closed
dualfroz wants to merge 1 commit into
tobgu:masterfrom
dualfroz:dualfroz/pdeque-maxlen-preserve
Closed

dualfroz wants to merge 1 commit into
tobgu:masterfrom
dualfroz:dualfroz/pdeque-maxlen-preserve

Conversation

@dualfroz

@dualfroz dualfroz commented Sep 5, 2026

Copy link
Copy Markdown

Problem

A bounded PDeque (created with maxlen) loses its bound when passed through
remove() or reverse():

from pyrsistent import pdeque
d = pdeque([1, 2, 3], maxlen=3)
d.remove(2).maxlen     # None  -- expected 3
d.reverse().maxlen     # None  -- expected 3

Because the bound is silently dropped, the result no longer enforces its
maximum length, so subsequent appends can grow it past the declared maxlen:

pdeque([1, 2, 3], maxlen=3).remove(2).append(4).append(5)
# pdeque([1, 3, 4, 5])   -- 4 elements in a maxlen=3 deque

Root cause

pyrsistent/_pdeque.py. PDeque.__new__ accepts maxlen=None as a default.
Every structural method threads self._maxlen into the new instance
(pop, popleft, append, extend, ...), except two:

  • remove() built its results with PDeque(..., self._length - 1), omitting
    the maxlen argument (both the left-list and the right-list branches).
  • reverse() built its result with PDeque(self._right_list, self._left_list, self._length), omitting maxlen.

Both therefore defaulted maxlen to None.

Fix

Pass self._maxlen through in both methods, matching every other structural
operation on PDeque.

Test

tests/deque_test.py::test_remove_preserves_maxlen and
::test_reverse_preserves_maxlen assert the returned deque keeps maxlen == 3
and stays bounded across a following append. (PDeque.__eq__ compares only the
elements and ignores maxlen, so the tests check .maxlen directly.)

Both fail on the current master (assert None == 3) and pass with the fix.

PDeque threads its maxlen into every structural result except remove()
and reverse(), which constructed the new PDeque without the maxlen
argument and thus defaulted it to None. A bounded deque therefore became
unbounded after either operation, so later appends could grow it past
its declared maximum length.

Pass self._maxlen through in both methods to match the other operations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant