From dbbe0907598f444f3f5348dfe03d2a58ebbdf25f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 22 Jul 2019 11:00:33 +0300 Subject: [PATCH 1/3] bpo-37648: Fixed minor inconsistency in some __contains__. The collection's item is now always at the left and the needle is on the right of ==. --- .../2019-07-22-11-05-05.bpo-37648.6TY2L-.rst | 3 +++ Modules/_asynciomodule.c | 6 +++--- Modules/_ssl.c | 3 +-- Objects/abstract.c | 2 +- Objects/dictobject.c | 2 +- Objects/listobject.c | 3 +-- Objects/tupleobject.c | 3 +-- 7 files changed, 11 insertions(+), 11 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst b/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst new file mode 100644 index 00000000000000..e97533a4a39f61 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst @@ -0,0 +1,3 @@ +Fixed minor inconsistency in ``list.__contains__()``, ``tuple.__contains__`` +and few other places. The collection's item is now always at the left and +the needle is on the right of ``==``. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index e9e6c5682d62e9..4d503a418a2e64 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -937,7 +937,7 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn) ENSURE_FUTURE_ALIVE(self) if (self->fut_callback0 != NULL) { - int cmp = PyObject_RichCompareBool(fn, self->fut_callback0, Py_EQ); + int cmp = PyObject_RichCompareBool(self->fut_callback0, fn, Py_EQ); if (cmp == -1) { return NULL; } @@ -962,7 +962,7 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn) if (len == 1) { PyObject *cb_tup = PyList_GET_ITEM(self->fut_callbacks, 0); int cmp = PyObject_RichCompareBool( - fn, PyTuple_GET_ITEM(cb_tup, 0), Py_EQ); + PyTuple_GET_ITEM(cb_tup, 0), fn, Py_EQ); if (cmp == -1) { return NULL; } @@ -984,7 +984,7 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn) int ret; PyObject *item = PyList_GET_ITEM(self->fut_callbacks, i); Py_INCREF(item); - ret = PyObject_RichCompareBool(fn, PyTuple_GET_ITEM(item, 0), Py_EQ); + ret = PyObject_RichCompareBool(PyTuple_GET_ITEM(item, 0), fn, Py_EQ); if (ret == 0) { if (j < len) { PyList_SET_ITEM(newlist, j, item); diff --git a/Modules/_ssl.c b/Modules/_ssl.c index da30cbb758e27e..3d54b844fe0732 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -5600,8 +5600,7 @@ list_contains(PyListObject *a, PyObject *el) int cmp; for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i) - cmp = PyObject_RichCompareBool(el, PyList_GET_ITEM(a, i), - Py_EQ); + cmp = PyObject_RichCompareBool(PyList_GET_ITEM(a, i), el, Py_EQ); return cmp; } diff --git a/Objects/abstract.c b/Objects/abstract.c index db1c3064db6f1f..f93d73fa7571ab 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -2016,7 +2016,7 @@ _PySequence_IterSearch(PyObject *seq, PyObject *obj, int operation) break; } - cmp = PyObject_RichCompareBool(obj, item, Py_EQ); + cmp = PyObject_RichCompareBool(item, obj, Py_EQ); Py_DECREF(item); if (cmp < 0) goto Fail; diff --git a/Objects/dictobject.c b/Objects/dictobject.c index b6205d93ca1408..f168ad5d2f00de 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -4392,7 +4392,7 @@ dictitems_contains(_PyDictViewObject *dv, PyObject *obj) return 0; } Py_INCREF(found); - result = PyObject_RichCompareBool(value, found, Py_EQ); + result = PyObject_RichCompareBool(found, value, Py_EQ); Py_DECREF(found); return result; } diff --git a/Objects/listobject.c b/Objects/listobject.c index d012ab933a9eb5..cea9b24a3b2fb6 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -449,8 +449,7 @@ list_contains(PyListObject *a, PyObject *el) int cmp; for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i) - cmp = PyObject_RichCompareBool(el, PyList_GET_ITEM(a, i), - Py_EQ); + cmp = PyObject_RichCompareBool(PyList_GET_ITEM(a, i), el, Py_EQ); return cmp; } diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index fc2d2742dd2ca6..aeaf845d74cfa1 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -403,8 +403,7 @@ tuplecontains(PyTupleObject *a, PyObject *el) int cmp; for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i) - cmp = PyObject_RichCompareBool(el, PyTuple_GET_ITEM(a, i), - Py_EQ); + cmp = PyObject_RichCompareBool(PyTuple_GET_ITEM(a, i), el, Py_EQ); return cmp; } From 502d26fac52cc8d7ab837b501c61514fee9cc067 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 27 Jul 2019 12:58:46 +0300 Subject: [PATCH 2/3] Add some tests. --- Lib/test/list_tests.py | 16 ++++++++++++++++ Lib/test/seq_tests.py | 30 +++++++++++++++++++++--------- Lib/test/support/__init__.py | 7 +++++++ Lib/test/test_iter.py | 24 +++++++++++++++++++++++- 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/Lib/test/list_tests.py b/Lib/test/list_tests.py index 40316de220fac2..eb6c840e2c1aee 100644 --- a/Lib/test/list_tests.py +++ b/Lib/test/list_tests.py @@ -7,6 +7,8 @@ from functools import cmp_to_key from test import support, seq_tests +from test.support import NONE +from unittest.mock import ANY class CommonTest(seq_tests.CommonTest): @@ -329,6 +331,20 @@ def test_remove(self): self.assertRaises(TypeError, a.remove) + a = self.type2test([1, 2]) + self.assertRaises(ValueError, a.remove, NONE) + self.assertEqual(a, [1, 2]) + a.remove(ANY) + self.assertEqual(a, [2]) + a = self.type2test([ANY]) + a.remove(1) + self.assertEqual(a, []) + a = self.type2test([ANY]) + a.remove(NONE) + self.assertEqual(a, []) + a = self.type2test([NONE]) + self.assertRaises(ValueError, a.remove, ANY) + class BadExc(Exception): pass diff --git a/Lib/test/seq_tests.py b/Lib/test/seq_tests.py index 65b110ef7818da..413b45854ea86c 100644 --- a/Lib/test/seq_tests.py +++ b/Lib/test/seq_tests.py @@ -6,6 +6,8 @@ import sys import pickle from test import support +from test.support import NONE +from unittest.mock import ANY # Various iterables # This is used for checking the constructor (here and in test_deque.py) @@ -221,15 +223,15 @@ def test_contains(self): self.assertRaises(TypeError, u.__contains__) def test_contains_fake(self): - class AllEq: - # Sequences must use rich comparison against each item - # (unless "is" is true, or an earlier item answered) - # So instances of AllEq must be found in all non-empty sequences. - def __eq__(self, other): - return True - __hash__ = None # Can't meet hash invariant requirements - self.assertNotIn(AllEq(), self.type2test([])) - self.assertIn(AllEq(), self.type2test([1])) + # Sequences must use rich comparison against each item + # (unless "is" is true, or an earlier item answered) + # So ANY must be found in all non-empty sequences. + self.assertNotIn(ANY, self.type2test([])) + self.assertIn(ANY, self.type2test([1])) + self.assertIn(1, self.type2test([ANY])) + self.assertNotIn(NONE, self.type2test([])) + self.assertNotIn(ANY, self.type2test([NONE])) + self.assertIn(NONE, self.type2test([ANY])) def test_contains_order(self): # Sequences must test in-order. If a rich comparison has side @@ -350,6 +352,11 @@ def test_count(self): self.assertEqual(a.count(1), 3) self.assertEqual(a.count(3), 0) + self.assertEqual(a.count(ANY), 9) + self.assertEqual(self.type2test([ANY, ANY]).count(1), 2) + self.assertEqual(self.type2test([ANY, ANY]).count(NONE), 2) + self.assertEqual(self.type2test([NONE, NONE]).count(ANY), 0) + self.assertRaises(TypeError, a.count) class BadExc(Exception): @@ -378,6 +385,11 @@ def test_index(self): self.assertEqual(u.index(0, 3, 4), 3) self.assertRaises(ValueError, u.index, 2, 0, -10) + self.assertEqual(u.index(ANY), 0) + self.assertEqual(self.type2test([ANY, ANY]).index(1), 0) + self.assertEqual(self.type2test([ANY, ANY]).index(NONE), 0) + self.assertRaises(ValueError, self.type2test([NONE, NONE]).index, ANY) + self.assertRaises(TypeError, u.index) class BadExc(Exception): diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 9efb3d91705ff0..e2f195009235a2 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -3093,6 +3093,13 @@ def __fspath__(self): return self.path +class NoneEq: + def __eq__(self, other): + return False + +NONE = NoneEq() + + def maybe_get_event_loop_policy(): """Return the global event loop policy if one is set, else return None.""" return asyncio.events._event_loop_policy diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py index 542b28419e2c8e..2be0e7e9fc4bbe 100644 --- a/Lib/test/test_iter.py +++ b/Lib/test/test_iter.py @@ -3,7 +3,8 @@ import sys import unittest from test.support import run_unittest, TESTFN, unlink, cpython_only -from test.support import check_free_after_iterating +from test.support import check_free_after_iterating, NONE +from unittest.mock import ANY import pickle import collections.abc @@ -41,6 +42,14 @@ def __init__(self, n): def __iter__(self): return BasicIterClass(self.n) +class IteratorProxyClass: + def __init__(self, i): + self.i = i + def __next__(self): + return next(self.i) + def __iter__(self): + return self + class SequenceClass: def __init__(self, n): self.n = n @@ -50,6 +59,12 @@ def __getitem__(self, i): else: raise IndexError +class SequenceProxyClass: + def __init__(self, s): + self.s = s + def __getitem__(self, i): + return self.s[i] + class UnlimitedSequenceClass: def __getitem__(self, i): return i @@ -635,6 +650,13 @@ def test_in_and_not_in(self): for i in "abc", -1, 5, 42.42, (3, 4), [], {1: 1}, 3-12j, sc5: self.assertNotIn(i, sc5) + self.assertIn(ANY, IteratorProxyClass(iter([1]))) + self.assertIn(ANY, SequenceProxyClass([1])) + self.assertNotIn(ANY, IteratorProxyClass(iter([NONE]))) + self.assertNotIn(ANY, SequenceProxyClass([NONE])) + self.assertIn(NONE, IteratorProxyClass(iter([ANY]))) + self.assertIn(NONE, SequenceProxyClass([ANY])) + self.assertRaises(TypeError, lambda: 3 in 12) self.assertRaises(TypeError, lambda: 3 not in map) From 005545361160d711b839759b07b6baa7672f4b6d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 4 Aug 2019 13:45:11 +0300 Subject: [PATCH 3/3] Fixed wording in NEWS. --- .../2019-07-22-11-05-05.bpo-37648.6TY2L-.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst b/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst index e97533a4a39f61..3c11d3d6008e7c 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2019-07-22-11-05-05.bpo-37648.6TY2L-.rst @@ -1,3 +1,3 @@ -Fixed minor inconsistency in ``list.__contains__()``, ``tuple.__contains__`` -and few other places. The collection's item is now always at the left and -the needle is on the right of ``==``. +Fixed minor inconsistency in :meth:`list.__contains__`, +:meth:`tuple.__contains__` and a few other places. The collection's item is +now always at the left and the needle is on the right of ``==``.