Skip to content

Commit

Permalink
Switched SortedList base class to Sequence
Browse files Browse the repository at this point in the history
Since SortedList does not implement the MutableSequence interface (many
of MutableSequence's mixin methods return NotImplemented), it should not
derive from MutableSequence.

This change will allow type checkers to properly reject SortedList as an
argument to a function that takes MutableSequence.

This is a breaking change (albeit a minor one).
  • Loading branch information
pganssle committed Jan 17, 2020
1 parent 22dec80 commit 4148246
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 65 deletions.
8 changes: 4 additions & 4 deletions docs/introduction.rst
Original file line number Diff line number Diff line change
Expand Up @@ -166,19 +166,19 @@ at an index which is not supported by :class:`SortedList`.
>>> sl.reverse()
Traceback (most recent call last):
...
NotImplementedError: use ``reversed(sl)`` instead
AttributeError: use ``reversed(sl)`` instead
>>> sl.append('f')
Traceback (most recent call last):
...
NotImplementedError: use ``sl.add(value)`` instead
AttributeError: use ``sl.add(value)`` instead
>>> sl.extend(['f', 'g', 'h'])
Traceback (most recent call last):
...
NotImplementedError: use ``sl.update(values)`` instead
AttributeError: use ``sl.update(values)`` instead
>>> sl.insert(5, 'f')
Traceback (most recent call last):
...
NotImplementedError: use ``sl.add(value)`` instead
AttributeError: use ``sl.add(value)`` instead

Comparison with :class:`SortedList` uses lexicographical ordering as with other
sequence types.
Expand Down
4 changes: 0 additions & 4 deletions docs/sortedlist.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ SortedList
.. automethod:: __repr__
.. automethod:: _check
.. automethod:: _reset
.. automethod:: append
.. automethod:: extend
.. automethod:: insert
.. automethod:: reverse
.. automethod:: __setitem__


Expand Down
78 changes: 24 additions & 54 deletions sortedcontainers/sortedlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
###############################################################################

try:
from collections.abc import Sequence, MutableSequence
from collections.abc import Sequence
except ImportError:
from collections import Sequence, MutableSequence
from collections import Sequence

from functools import wraps
from sys import hexversion
Expand Down Expand Up @@ -82,7 +82,7 @@ def wrapper(self):
###############################################################################


class SortedList(MutableSequence):
class SortedList(Sequence):
"""Sorted list is a sorted mutable sequence.
Sorted list values are maintained in sorted order.
Expand Down Expand Up @@ -134,8 +134,14 @@ class SortedList(MutableSequence):
Sorted lists use lexicographical ordering semantics when compared to other
sequences.
Some methods of mutable sequences are not supported and will raise
not-implemented error.
.. versionchanged:: 3.0
SortedLists are mutable sequences but they do not implement the
:class:`collections.abc.MutableSequence` interface. In version 3.0, the
base class was switched to :class:`collections.abc.Sequence` and the
``append``, ``extend``, ``reverse`` and ``insert`` methods, which
previously raised :py:exc:`NotImplementedError` when called, were
removed.
"""
DEFAULT_LOAD_FACTOR = 1000
Expand Down Expand Up @@ -932,24 +938,6 @@ def __reversed__(self):
return chain.from_iterable(map(reversed, reversed(self._lists)))


def reverse(self):
"""Raise not-implemented error.
Sorted list maintains values in ascending sort order. Values may not be
reversed in-place.
Use ``reversed(sl)`` for an iterator over values in descending sort
order.
Implemented to override `MutableSequence.reverse` which provides an
erroneous default implementation.
:raises NotImplementedError: use ``reversed(sl)`` instead
"""
raise NotImplementedError('use ``reversed(sl)`` instead')


def islice(self, start=None, stop=None, reverse=False):
"""Return an iterator that slices sorted list from `start` to `stop`.
Expand Down Expand Up @@ -1273,38 +1261,20 @@ def copy(self):

__copy__ = copy

def __getattr__(self, key):
if key == 'append':
msg = 'use ``sl.add(value)`` instead'
elif key == 'extend':
msg = 'use ``sl.update(values)`` instead'
elif key == 'reverse':
msg = 'use ``reversed(sl)`` instead'
elif key == 'insert':
msg = 'use ``sl.add(value)`` instead'
else:
msg = "'%s' object has no attribute '%s'" % (type(self).__name__,
key)

def append(self, value):
"""Raise not-implemented error.
Implemented to override `MutableSequence.append` which provides an
erroneous default implementation.
:raises NotImplementedError: use ``sl.add(value)`` instead
"""
raise NotImplementedError('use ``sl.add(value)`` instead')


def extend(self, values):
"""Raise not-implemented error.
Implemented to override `MutableSequence.extend` which provides an
erroneous default implementation.
:raises NotImplementedError: use ``sl.update(values)`` instead
"""
raise NotImplementedError('use ``sl.update(values)`` instead')


def insert(self, index, value):
"""Raise not-implemented error.
:raises NotImplementedError: use ``sl.add(value)`` instead
"""
raise NotImplementedError('use ``sl.add(value)`` instead')
raise AttributeError(msg)


def pop(self, index=-1):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_coverage_sortedkeylist_modulo.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def test_reversed():

def test_reverse():
slt = SortedKeyList(range(10000), key=modulo)
with pytest.raises(NotImplementedError):
with pytest.raises(AttributeError):
slt.reverse()

def test_islice():
Expand Down
2 changes: 1 addition & 1 deletion tests/test_coverage_sortedkeylist_negate.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def test_reversed():

def test_reverse():
slt = SortedKeyList(range(10000), key=negate)
with pytest.raises(NotImplementedError):
with pytest.raises(AttributeError):
slt.reverse()

def test_islice():
Expand Down
33 changes: 32 additions & 1 deletion tests/test_coverage_sortedlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
from itertools import chain
import pytest

try:
from collections import abc
except ImportError:
import collections as abc

if hexversion < 0x03000000:
from itertools import izip as zip
range = xrange
Expand Down Expand Up @@ -266,7 +271,7 @@ def test_reversed():

def test_reverse():
slt = SortedList(range(10000))
with pytest.raises(NotImplementedError):
with pytest.raises(AttributeError):
slt.reverse()

def test_islice():
Expand Down Expand Up @@ -604,3 +609,29 @@ def test_check():
slt._len = 5
with pytest.raises(AssertionError):
slt._check()


@pytest.mark.parametrize("base_class_name",
["Sequence", "Reversible", "Iterable",
"Sized", "Collection"])
def test_abstract_base_classes(base_class_name):
# Some of these were introduced in later versions of Python
base_class = getattr(abc, base_class_name, None)
if base_class is None:
pytest.skip("%s introduced in a later version of Python" %
base_class_name)

sl = SortedList()
assert isinstance(sl, base_class)


def test_not_mutable_sequence_instance():
"""
SortedList does not implement the MutableSequence interface, but has custom
behavior for all the methods of a MutableSquence, so we want to make sure
that it still fails an `isinstance` check.
"""

sl = SortedList()
assert not isinstance(sl, abc.MutableSequence)

0 comments on commit 4148246

Please sign in to comment.