Skip to content

bpo-37648: Fixed minor inconsistency in some __contains__. #14904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Doc/library/test.rst
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,12 @@ The :mod:`test.support` module defines the following constants:
Object that is equal to anything. Used to test mixed type comparison.


.. data:: NEVER_EQ

Object that is not equal to anything (even to :data:`ALWAYS_EQ`).
Used to test mixed type comparison.


.. data:: LARGEST

Object that is greater than anything (except itself).
Expand Down
15 changes: 15 additions & 0 deletions Lib/test/list_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from functools import cmp_to_key

from test import support, seq_tests
from test.support import ALWAYS_EQ, NEVER_EQ


class CommonTest(seq_tests.CommonTest):
Expand Down Expand Up @@ -329,6 +330,20 @@ def test_remove(self):

self.assertRaises(TypeError, a.remove)

a = self.type2test([1, 2])
self.assertRaises(ValueError, a.remove, NEVER_EQ)
self.assertEqual(a, [1, 2])
a.remove(ALWAYS_EQ)
self.assertEqual(a, [2])
a = self.type2test([ALWAYS_EQ])
a.remove(1)
self.assertEqual(a, [])
a = self.type2test([ALWAYS_EQ])
a.remove(NEVER_EQ)
self.assertEqual(a, [])
a = self.type2test([NEVER_EQ])
self.assertRaises(ValueError, a.remove, ALWAYS_EQ)

class BadExc(Exception):
pass

Expand Down
29 changes: 20 additions & 9 deletions Lib/test/seq_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import sys
import pickle
from test import support
from test.support import ALWAYS_EQ, NEVER_EQ

# Various iterables
# This is used for checking the constructor (here and in test_deque.py)
Expand Down Expand Up @@ -221,15 +222,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 ALWAYS_EQ must be found in all non-empty sequences.
self.assertNotIn(ALWAYS_EQ, self.type2test([]))
self.assertIn(ALWAYS_EQ, self.type2test([1]))
self.assertIn(1, self.type2test([ALWAYS_EQ]))
self.assertNotIn(NEVER_EQ, self.type2test([]))
self.assertNotIn(ALWAYS_EQ, self.type2test([NEVER_EQ]))
self.assertIn(NEVER_EQ, self.type2test([ALWAYS_EQ]))

def test_contains_order(self):
# Sequences must test in-order. If a rich comparison has side
Expand Down Expand Up @@ -350,6 +351,11 @@ def test_count(self):
self.assertEqual(a.count(1), 3)
self.assertEqual(a.count(3), 0)

self.assertEqual(a.count(ALWAYS_EQ), 9)
self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).count(1), 2)
self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).count(NEVER_EQ), 2)
self.assertEqual(self.type2test([NEVER_EQ, NEVER_EQ]).count(ALWAYS_EQ), 0)

self.assertRaises(TypeError, a.count)

class BadExc(Exception):
Expand Down Expand Up @@ -378,6 +384,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(ALWAYS_EQ), 0)
self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).index(1), 0)
self.assertEqual(self.type2test([ALWAYS_EQ, ALWAYS_EQ]).index(NEVER_EQ), 0)
self.assertRaises(ValueError, self.type2test([NEVER_EQ, NEVER_EQ]).index, ALWAYS_EQ)

self.assertRaises(TypeError, u.index)

class BadExc(Exception):
Expand Down
13 changes: 12 additions & 1 deletion Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"run_with_locale", "swap_item",
"swap_attr", "Matcher", "set_memlimit", "SuppressCrashReport", "sortdict",
"run_with_tz", "PGO", "missing_compiler_executable", "fd_count",
"ALWAYS_EQ", "LARGEST", "SMALLEST"
"ALWAYS_EQ", "NEVER_EQ", "LARGEST", "SMALLEST"
]

class Error(Exception):
Expand Down Expand Up @@ -3115,6 +3115,17 @@ def __ne__(self, other):

ALWAYS_EQ = _ALWAYS_EQ()

class _NEVER_EQ:
"""
Object that is not equal to anything.
"""
def __eq__(self, other):
return False
def __ne__(self, other):
return True

NEVER_EQ = _NEVER_EQ()

@functools.total_ordering
class _LARGEST:
"""
Expand Down
23 changes: 22 additions & 1 deletion Lib/test/test_iter.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
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, ALWAYS_EQ, NEVER_EQ
import pickle
import collections.abc

Expand Down Expand Up @@ -41,6 +41,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
Expand All @@ -50,6 +58,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
Expand Down Expand Up @@ -635,6 +649,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(ALWAYS_EQ, IteratorProxyClass(iter([1])))
self.assertIn(ALWAYS_EQ, SequenceProxyClass([1]))
self.assertNotIn(ALWAYS_EQ, IteratorProxyClass(iter([NEVER_EQ])))
self.assertNotIn(ALWAYS_EQ, SequenceProxyClass([NEVER_EQ]))
self.assertIn(NEVER_EQ, IteratorProxyClass(iter([ALWAYS_EQ])))
self.assertIn(NEVER_EQ, SequenceProxyClass([ALWAYS_EQ]))

self.assertRaises(TypeError, lambda: 3 in 12)
self.assertRaises(TypeError, lambda: 3 not in map)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
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 ``==``.
6 changes: 3 additions & 3 deletions Modules/_asynciomodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion Objects/abstract.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 1 addition & 2 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 1 addition & 2 deletions Objects/tupleobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down