# HG changeset patch # User Andrew Barnert # Date 1452031303 28800 # Tue Jan 05 14:01:43 2016 -0800 # Branch issue25864 # Node ID 48c57af8da0c4bf7de7961cd1819f81515be9e8b # Parent 164b564e3c1ab51fbe1c2c29ca019de7fbf6ffc4 Issue #25958: Make `__spam__ = None` consistent Setting a special method `__spam__ = None` in a class has always not only blocked the corresponding feature with a `TypeError`. For example, if you set `__iter__ = None`, calling `iter()` on an instance of your class will raise a `TypeError`, without falling back to the old-style sequence protocol (`__getitem__`). However, there were a few problems: 1. Only a few of the ABCs in `collections.abc` with subclass hooks understood `None`. For example, `__iter__ = None` makes your class a virtual instance of `Iterable`, when you intended exactly the reverse. 2. In most cases, the `TypeError` has a useless message, like `'NoneType' object is not callable`, instead of something nicer like `'MyClass' object is not iterable`. 3. There was nothing in the documentation explaining this behavior. 4. A few non-internal "protocols" don't follow the same rule. In particular, `pickle`/`deepcopy` use `None` rather than `NotImplemented` to trigger fallback, meaning they can't use `None` to block fallback. This patch fixes the first three problems, and also includes two other related changes: * All `collections.abc.*.__subclasshook__` methods now use the same code to search for method implementations, and that code treats `None` as blocking. * The `__iter__`, `__reversed__`, and `__contains__` methods now use a custom `TypeError` message for `None`, as these (along with `__hash__`, which already had one) are the ones most likely to come up in realistic code. * The Data Model section of the reference documents `None` blocking (with footnotes for uncommon but surprising cases, like `__rspam__`). * Some new unit tests are added to the test suite. * `collections.abc.Mapping.__reversed__ = None`. (#25864) * `collections.abc.Reversible` added. (#25987) diff -r 164b564e3c1a -r 48c57af8da0c Doc/library/collections.abc.rst --- a/Doc/library/collections.abc.rst Sun Jan 03 17:58:24 2016 -0800 +++ b/Doc/library/collections.abc.rst Tue Jan 05 14:01:43 2016 -0800 @@ -42,10 +42,11 @@ :class:`Iterator` :class:`Iterable` ``__next__`` ``__iter__`` :class:`Generator` :class:`Iterator` ``send``, ``throw`` ``close``, ``__iter__``, ``__next__`` :class:`Sized` ``__len__`` +:class:`Reversible` :class:`Iterable` ``__reversed__`` :class:`Callable` ``__call__`` :class:`Sequence` :class:`Sized`, ``__getitem__``, ``__contains__``, ``__iter__``, ``__reversed__``, - :class:`Iterable`, ``__len__`` ``index``, and ``count`` + :class:`Reversible`, ``__len__`` ``index``, and ``count`` :class:`Container` :class:`MutableSequence` :class:`Sequence` ``__getitem__``, Inherited :class:`Sequence` methods and @@ -116,6 +117,13 @@ .. versionadded:: 3.5 +.. class:: Reversible + + ABC for iterable classes that also provide the :meth:`__reversed__` + method. + + .. versionadded:: 3.6 + .. class:: Sequence MutableSequence diff -r 164b564e3c1a -r 48c57af8da0c Doc/reference/datamodel.rst --- a/Doc/reference/datamodel.rst Sun Jan 03 17:58:24 2016 -0800 +++ b/Doc/reference/datamodel.rst Tue Jan 05 14:01:43 2016 -0800 @@ -1064,6 +1064,12 @@ operation raise an exception when no appropriate method is defined (typically :exc:`AttributeError` or :exc:`TypeError`). +Setting a special method to ``None`` indicates that the corresponding +operation is not available. For example, if a class sets +:meth:`__iter__` to ``None``, the class is not iterable, so calling +:func:`iter` on its instances will raise a :exc:`TypeError` (without +falling back to :meth:`__getitem__`). [#]_ + When implementing a class that emulates any built-in type, it is important that the emulation only be implemented to the degree that it makes sense for the object being modelled. For example, some sequences may work well with retrieval @@ -2055,7 +2061,7 @@ (``+``, ``-``, ``*``, ``@``, ``/``, ``//``, ``%``, :func:`divmod`, :func:`pow`, ``**``, ``<<``, ``>>``, ``&``, ``^``, ``|``) with reflected (swapped) operands. These functions are only called if the left operand does - not support the corresponding operation and the operands are of different + not support the corresponding operation [#]_ and the operands are of different types. [#]_ For instance, to evaluate the expression ``x - y``, where *y* is an instance of a class that has an :meth:`__rsub__` method, ``y.__rsub__(x)`` is called if ``x.__sub__(y)`` returns *NotImplemented*. @@ -2423,6 +2429,17 @@ controlled conditions. It generally isn't a good idea though, since it can lead to some very strange behaviour if it is handled incorrectly. +.. [#] The :meth:`__hash__`, :meth:`__iter__`, :meth:`__reversed__`, and + :meth:`__contains__` methods have special handling for this; others + will still raise a :exc:`TypeError`, but may do so by relying on + the behavior that ``None`` is not callable. + +.. [#] "Does not support" here means that the class has no such method, or + the method returns ``NotImplemented``. Do not assign the method to + ``None`` if you want to force fallback to the right operand's reflected + method--that will instead have the opposite effect of explicitly + *blocking* such fallback. + .. [#] For operands of the same type, it is assumed that if the non-reflected method (such as :meth:`__add__`) fails the operation is not supported, which is why the reflected method is not called. diff -r 164b564e3c1a -r 48c57af8da0c Lib/_collections_abc.py --- a/Lib/_collections_abc.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/_collections_abc.py Tue Jan 05 14:01:43 2016 -0800 @@ -11,7 +11,7 @@ __all__ = ["Awaitable", "Coroutine", "AsyncIterable", "AsyncIterator", "Hashable", "Iterable", "Iterator", "Generator", - "Sized", "Container", "Callable", + "Sized", "Container", "Reversible", "Callable", "Set", "MutableSet", "Mapping", "MutableMapping", "MappingView", "KeysView", "ItemsView", "ValuesView", @@ -62,6 +62,18 @@ ### ONE-TRICK PONIES ### +def _check_methods(C, *methods): + mro = C.__mro__ + for method in methods: + for B in mro: + if method in B.__dict__: + if B.__dict__[method] is None: + return NotImplemented + break + else: + return NotImplemented + return True + class Hashable(metaclass=ABCMeta): __slots__ = () @@ -73,11 +85,7 @@ @classmethod def __subclasshook__(cls, C): if cls is Hashable: - for B in C.__mro__: - if "__hash__" in B.__dict__: - if B.__dict__["__hash__"]: - return True - break + return _check_methods(C, "__hash__") return NotImplemented @@ -92,11 +100,7 @@ @classmethod def __subclasshook__(cls, C): if cls is Awaitable: - for B in C.__mro__: - if "__await__" in B.__dict__: - if B.__dict__["__await__"]: - return True - break + return _check_methods(C, "__await__") return NotImplemented @@ -137,14 +141,7 @@ @classmethod def __subclasshook__(cls, C): if cls is Coroutine: - mro = C.__mro__ - for method in ('__await__', 'send', 'throw', 'close'): - for base in mro: - if method in base.__dict__: - break - else: - return NotImplemented - return True + return _check_methods(C, '__await__', 'send', 'throw', 'close') return NotImplemented @@ -162,8 +159,7 @@ @classmethod def __subclasshook__(cls, C): if cls is AsyncIterable: - if any("__aiter__" in B.__dict__ for B in C.__mro__): - return True + return _check_methods(C, "__aiter__") return NotImplemented @@ -182,9 +178,7 @@ @classmethod def __subclasshook__(cls, C): if cls is AsyncIterator: - if (any("__anext__" in B.__dict__ for B in C.__mro__) and - any("__aiter__" in B.__dict__ for B in C.__mro__)): - return True + return _check_methods(C, "__anext__", "__aiter__") return NotImplemented @@ -200,8 +194,7 @@ @classmethod def __subclasshook__(cls, C): if cls is Iterable: - if any("__iter__" in B.__dict__ for B in C.__mro__): - return True + return _check_methods(C, "__iter__") return NotImplemented @@ -220,9 +213,7 @@ @classmethod def __subclasshook__(cls, C): if cls is Iterator: - if (any("__next__" in B.__dict__ for B in C.__mro__) and - any("__iter__" in B.__dict__ for B in C.__mro__)): - return True + return _check_methods(C, "__next__", "__iter__") return NotImplemented Iterator.register(bytes_iterator) @@ -283,14 +274,8 @@ @classmethod def __subclasshook__(cls, C): if cls is Generator: - mro = C.__mro__ - for method in ('__iter__', '__next__', 'send', 'throw', 'close'): - for base in mro: - if method in base.__dict__: - break - else: - return NotImplemented - return True + return _check_methods(C, '__iter__', '__next__', + 'send', 'throw', 'close') return NotImplemented @@ -308,8 +293,7 @@ @classmethod def __subclasshook__(cls, C): if cls is Sized: - if any("__len__" in B.__dict__ for B in C.__mro__): - return True + return _check_methods(C, "__len__") return NotImplemented @@ -324,8 +308,7 @@ @classmethod def __subclasshook__(cls, C): if cls is Container: - if any("__contains__" in B.__dict__ for B in C.__mro__): - return True + return _check_methods(C, "__contains__") return NotImplemented @@ -340,8 +323,23 @@ @classmethod def __subclasshook__(cls, C): if cls is Callable: - if any("__call__" in B.__dict__ for B in C.__mro__): - return True + return _check_methods(C, "__call__") + return NotImplemented + + +class Reversible(Iterable): + + __slots__ = () + + @abstractmethod + def __reversed__(self): + while False: + yield None + + @classmethod + def __subclasshook__(cls, C): + if cls is Reversible: + return _check_methods(C, "__reversed__", "__iter__") return NotImplemented @@ -621,6 +619,8 @@ return NotImplemented return dict(self.items()) == dict(other.items()) + __reversed__ = None + Mapping.register(mappingproxy) @@ -794,7 +794,7 @@ ### SEQUENCES ### -class Sequence(Sized, Iterable, Container): +class Sequence(Sized, Reversible, Container): """All the operations on a read-only sequence. diff -r 164b564e3c1a -r 48c57af8da0c Lib/test/test_augassign.py --- a/Lib/test/test_augassign.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/test/test_augassign.py Tue Jan 05 14:01:43 2016 -0800 @@ -83,6 +83,10 @@ def __iadd__(self, val): return aug_test3(self.val + val) + class aug_test4(aug_test3): + "Blocks inheritance, and fallback to __add__" + __iadd__ = None + x = aug_test(1) y = x x += 10 @@ -106,6 +110,9 @@ self.assertTrue(y is not x) self.assertEqual(x.val, 13) + x = aug_test4(4) + with self.assertRaises(TypeError): + x += 10 def testCustomMethods2(test_self): output = [] diff -r 164b564e3c1a -r 48c57af8da0c Lib/test/test_binop.py --- a/Lib/test/test_binop.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/test/test_binop.py Tue Jan 05 14:01:43 2016 -0800 @@ -388,6 +388,34 @@ self.assertEqual(op_sequence(eq, B, V), ['B.__eq__', 'V.__eq__']) self.assertEqual(op_sequence(le, B, V), ['B.__le__', 'V.__ge__']) +class E(object): + """Class that can test equality""" + def __eq__(self, other): + return True + +class S(E): + """Subclass of E that should fail""" + __eq__ = None +class F(object): + """Independent class that should fall back""" + +class X(object): + """Independent class that should fail""" + __eq__ = None + +class FallbackBlockingTests(unittest.TestCase): + def test_fallback_blocking(self): + e, f, s, x = E(), F(), S(), X() + self.assertEqual(e, e) + self.assertEqual(e, f) + self.assertEqual(f, e) + # left operand is checked first + self.assertEqual(e, x) + self.assertRaises(TypeError, eq, x, e) + # S is a subclass, so it's always checked first + self.assertRaises(TypeError, eq, e, s) + self.assertRaises(TypeError, eq, s, e) + if __name__ == "__main__": unittest.main() diff -r 164b564e3c1a -r 48c57af8da0c Lib/test/test_collections.py --- a/Lib/test/test_collections.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/test/test_collections.py Tue Jan 05 14:01:43 2016 -0800 @@ -21,7 +21,7 @@ from collections import deque from collections.abc import Awaitable, Coroutine, AsyncIterator, AsyncIterable from collections.abc import Hashable, Iterable, Iterator, Generator -from collections.abc import Sized, Container, Callable +from collections.abc import Sized, Container, Reversible, Callable from collections.abc import Set, MutableSet from collections.abc import Mapping, MutableMapping, KeysView, ItemsView from collections.abc import Sequence, MutableSequence @@ -688,6 +688,64 @@ self.assertFalse(issubclass(str, I)) self.validate_abstract_methods(Iterable, '__iter__') self.validate_isinstance(Iterable, '__iter__') + # Check None blocking + class J: + def __iter__(self): return iter([]) + class K(J): + __iter__ = None + self.assertTrue(issubclass(J, Iterable)) + self.assertTrue(isinstance(J(), Iterable)) + self.assertFalse(issubclass(K, Iterable)) + self.assertFalse(isinstance(K(), Iterable)) + + def test_Reversible(self): + # Check some non-iterables + non_iterables = [None, 42, 3.14, 1j] + for x in non_iterables: + self.assertNotIsInstance(x, Reversible) + self.assertFalse(issubclass(type(x), Reversible), repr(type(x))) + # Check some non-reversible iterables + non_reversibles = [set(), frozenset(), dict(), dict().keys(), + dict().items(), dict().values(), + Counter(), Counter().keys(), Counter().items(), + Counter().values(), (lambda: (yield))(), + (x for x in []), iter([]), reversed([]) + ] + for x in non_reversibles: + self.assertNotIsInstance(x, Reversible) + self.assertFalse(issubclass(type(x), Reversible), repr(type(x))) + # Check some reversible iterables + samples = [bytes(), str(), + tuple(), list(), OrderedDict(), OrderedDict().keys(), + OrderedDict().items(), OrderedDict().values(), + ] + for x in samples: + self.assertIsInstance(x, Reversible) + self.assertTrue(issubclass(type(x), Reversible), repr(type(x))) + # Check direct subclassing + class R(Reversible): + def __iter__(self): + return super().__iter__() + def __reversed__(self): + return super().__reversed__() + self.assertEqual(list(R()), []) + self.assertEqual(list(reversed(R())), []) + self.assertFalse(issubclass(str, R)) + self.validate_abstract_methods(Reversible, '__reversed__', '__iter__') + # Check None blocking + class J: + def __iter__(self): return iter([]) + def __reversed__(self): return reversed([]) + class K(J): + __iter__ = None + class L(J): + __reversed__ = None + self.assertTrue(issubclass(J, Reversible)) + self.assertTrue(isinstance(J(), Reversible)) + self.assertFalse(issubclass(K, Reversible)) + self.assertFalse(isinstance(K(), Reversible)) + self.assertFalse(issubclass(L, Reversible)) + self.assertFalse(isinstance(L(), Reversible)) def test_Iterator(self): non_samples = [None, 42, 3.14, 1j, b"", "", (), [], {}, set()] @@ -1219,6 +1277,7 @@ def __iter__(self): return iter(()) self.validate_comparison(MyMapping()) + self.assertRaises(TypeError, reversed, MyMapping()) def test_MutableMapping(self): for sample in [dict]: diff -r 164b564e3c1a -r 48c57af8da0c Lib/test/test_contains.py --- a/Lib/test/test_contains.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/test/test_contains.py Tue Jan 05 14:01:43 2016 -0800 @@ -84,6 +84,27 @@ self.assertTrue(container == constructor(values)) self.assertTrue(container == container) + def test_block_fallback(self): + # blocking fallback with __contains__ = None + class ByContains(object): + def __contains__(self, other): + return False + c = ByContains() + class BlockContains(ByContains): + """Is not a container + + This class is a perfectly good iterable, and inherits from + a perfectly good container, but __contains__ = None blocks + both of these from working.""" + def __iter__(self): + while False: + yield None + __contains__ = None + bc = BlockContains() + self.assertFalse(0 in c) + self.assertFalse(0 in list(bc)) + self.assertRaises(TypeError, lambda: 0 in bc) + if __name__ == '__main__': unittest.main() diff -r 164b564e3c1a -r 48c57af8da0c Lib/test/test_enumerate.py --- a/Lib/test/test_enumerate.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/test/test_enumerate.py Tue Jan 05 14:01:43 2016 -0800 @@ -232,6 +232,13 @@ ngi = NoGetItem() self.assertRaises(TypeError, reversed, ngi) + class Blocked(object): + def __getitem__(self): return 1 + def __len__(self): return 2 + __reversed__ = None + b = Blocked() + self.assertRaises(TypeError, reversed, b) + def test_pickle(self): for data in 'abc', range(5), tuple(enumerate('abc')), range(1,17,5): self.check_pickle(reversed(data), list(data)[::-1]) diff -r 164b564e3c1a -r 48c57af8da0c Lib/test/test_functools.py --- a/Lib/test/test_functools.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/test/test_functools.py Tue Jan 05 14:01:43 2016 -0800 @@ -1435,7 +1435,8 @@ m = mro(D, bases) self.assertEqual(m, [D, c.MutableSequence, c.Sequence, c.defaultdict, dict, c.MutableMapping, - c.Mapping, c.Sized, c.Iterable, c.Container, + c.Mapping, c.Sized, c.Reversible, + c.Iterable, c.Container, object]) # Container and Callable are registered on different base classes and diff -r 164b564e3c1a -r 48c57af8da0c Lib/test/test_iter.py --- a/Lib/test/test_iter.py Sun Jan 03 17:58:24 2016 -0800 +++ b/Lib/test/test_iter.py Tue Jan 05 14:01:43 2016 -0800 @@ -53,6 +53,14 @@ def __getitem__(self, i): return i +class DefaultIterClass: + pass + +class NoIterClass: + def __getitem__(self, i): + return i + __iter__ = None + # Main test suite class TestCase(unittest.TestCase): @@ -944,6 +952,10 @@ self.assertEqual(next(it), 0) self.assertEqual(next(it), 1) + def test_error_iter(self): + for typ in (DefaultIterClass, NoIterClass): + self.assertRaises(TypeError, iter, typ()) + def test_main(): run_unittest(TestCase) diff -r 164b564e3c1a -r 48c57af8da0c Objects/enumobject.c --- a/Objects/enumobject.c Sun Jan 03 17:58:24 2016 -0800 +++ b/Objects/enumobject.c Tue Jan 05 14:01:43 2016 -0800 @@ -250,6 +250,12 @@ return NULL; reversed_meth = _PyObject_LookupSpecial(seq, &PyId___reversed__); + if (reversed_meth == Py_None) { + Py_DECREF(reversed_meth); + PyErr_SetString(PyExc_TypeError, + "argument to reversed() must be a sequence"); + return NULL; + } if (reversed_meth != NULL) { PyObject *res = PyObject_CallFunctionObjArgs(reversed_meth, NULL); Py_DECREF(reversed_meth); diff -r 164b564e3c1a -r 48c57af8da0c Objects/typeobject.c --- a/Objects/typeobject.c Sun Jan 03 17:58:24 2016 -0800 +++ b/Objects/typeobject.c Tue Jan 05 14:01:43 2016 -0800 @@ -5819,6 +5819,13 @@ _Py_IDENTIFIER(__contains__); func = lookup_maybe(self, &PyId___contains__); + if (func == Py_None) { + Py_DECREF(func); + PyErr_Format(PyExc_TypeError, + "argument of type '%.200s' is not a container", + Py_TYPE(self)->tp_name); + return -1; + } if (func != NULL) { args = PyTuple_Pack(1, value); if (args == NULL) @@ -6204,6 +6211,13 @@ _Py_IDENTIFIER(__iter__); func = lookup_method(self, &PyId___iter__); + if (func == Py_None) { + Py_DECREF(func); + PyErr_Format(PyExc_TypeError, + "'%.200s' object is not iterable", + Py_TYPE(self)->tp_name); + return NULL; + } if (func != NULL) { PyObject *args; args = res = PyTuple_New(0);