From 414824635e816fe391a4adeaf18ad40d351b2b67 Mon Sep 17 00:00:00 2001 From: Paul Ganssle Date: Fri, 17 Jan 2020 11:07:43 -0500 Subject: [PATCH] Switched SortedList base class to Sequence 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). --- docs/introduction.rst | 8 +-- docs/sortedlist.rst | 4 -- sortedcontainers/sortedlist.py | 78 +++++++-------------- tests/test_coverage_sortedkeylist_modulo.py | 2 +- tests/test_coverage_sortedkeylist_negate.py | 2 +- tests/test_coverage_sortedlist.py | 33 ++++++++- 6 files changed, 62 insertions(+), 65 deletions(-) diff --git a/docs/introduction.rst b/docs/introduction.rst index aa3ac8b4..5ed29f63 100644 --- a/docs/introduction.rst +++ b/docs/introduction.rst @@ -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. diff --git a/docs/sortedlist.rst b/docs/sortedlist.rst index 06466444..e34d573b 100644 --- a/docs/sortedlist.rst +++ b/docs/sortedlist.rst @@ -41,10 +41,6 @@ SortedList .. automethod:: __repr__ .. automethod:: _check .. automethod:: _reset - .. automethod:: append - .. automethod:: extend - .. automethod:: insert - .. automethod:: reverse .. automethod:: __setitem__ diff --git a/sortedcontainers/sortedlist.py b/sortedcontainers/sortedlist.py index 8b5699db..50a3c8e6 100644 --- a/sortedcontainers/sortedlist.py +++ b/sortedcontainers/sortedlist.py @@ -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 @@ -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. @@ -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 @@ -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`. @@ -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): diff --git a/tests/test_coverage_sortedkeylist_modulo.py b/tests/test_coverage_sortedkeylist_modulo.py index 1916054f..0e6563ab 100644 --- a/tests/test_coverage_sortedkeylist_modulo.py +++ b/tests/test_coverage_sortedkeylist_modulo.py @@ -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(): diff --git a/tests/test_coverage_sortedkeylist_negate.py b/tests/test_coverage_sortedkeylist_negate.py index 6c0287f8..7d5dc4c1 100644 --- a/tests/test_coverage_sortedkeylist_negate.py +++ b/tests/test_coverage_sortedkeylist_negate.py @@ -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(): diff --git a/tests/test_coverage_sortedlist.py b/tests/test_coverage_sortedlist.py index 7e009e11..b3b68f12 100644 --- a/tests/test_coverage_sortedlist.py +++ b/tests/test_coverage_sortedlist.py @@ -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 @@ -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(): @@ -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) +