Skip to content
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

bpo-17576: Strict __int__ and __index__ return types; operator.index always uses __index__ #13740

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions Lib/test/test_copy.py
Expand Up @@ -586,9 +586,7 @@ class I(int):
y = copy.deepcopy(x)
self.assertIsNot(y, x)
self.assertEqual(y, x)
self.assertIsNot(y.stop, x.stop)
self.assertEqual(y.stop, x.stop)
self.assertIsInstance(y.stop, I)

# _reconstruct()

Expand Down
76 changes: 38 additions & 38 deletions Lib/test/test_getargs2.py
Expand Up @@ -158,15 +158,15 @@ def test_b(self):
self.assertEqual(99, getargs_b(Index()))
self.assertEqual(0, getargs_b(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_b, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_b(BadIndex2()))
with self.assertRaises(TypeError):
getargs_b(BadIndex2())
self.assertEqual(0, getargs_b(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_b(Int()))
self.assertEqual(0, getargs_b(IntSubclass()))
self.assertRaises(TypeError, getargs_b, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_b(BadInt2()))
with self.assertRaises(TypeError):
getargs_b(BadInt2())
self.assertEqual(0, getargs_b(BadInt3()))

self.assertRaises(OverflowError, getargs_b, -1)
Expand All @@ -184,15 +184,15 @@ def test_B(self):
self.assertEqual(99, getargs_B(Index()))
self.assertEqual(0, getargs_B(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_B, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_B(BadIndex2()))
with self.assertRaises(TypeError):
getargs_B(BadIndex2())
self.assertEqual(0, getargs_B(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_B(Int()))
self.assertEqual(0, getargs_B(IntSubclass()))
self.assertRaises(TypeError, getargs_B, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_B(BadInt2()))
with self.assertRaises(TypeError):
getargs_B(BadInt2())
self.assertEqual(0, getargs_B(BadInt3()))

self.assertEqual(UCHAR_MAX, getargs_B(-1))
Expand All @@ -210,15 +210,15 @@ def test_H(self):
self.assertEqual(99, getargs_H(Index()))
self.assertEqual(0, getargs_H(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_H, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_H(BadIndex2()))
with self.assertRaises(TypeError):
getargs_H(BadIndex2())
self.assertEqual(0, getargs_H(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_H(Int()))
self.assertEqual(0, getargs_H(IntSubclass()))
self.assertRaises(TypeError, getargs_H, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_H(BadInt2()))
with self.assertRaises(TypeError):
getargs_H(BadInt2())
self.assertEqual(0, getargs_H(BadInt3()))

self.assertEqual(USHRT_MAX, getargs_H(-1))
Expand All @@ -237,15 +237,15 @@ def test_I(self):
self.assertEqual(99, getargs_I(Index()))
self.assertEqual(0, getargs_I(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_I, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_I(BadIndex2()))
with self.assertRaises(TypeError):
getargs_I(BadIndex2())
self.assertEqual(0, getargs_I(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_I(Int()))
self.assertEqual(0, getargs_I(IntSubclass()))
self.assertRaises(TypeError, getargs_I, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_I(BadInt2()))
with self.assertRaises(TypeError):
getargs_I(BadInt2())
self.assertEqual(0, getargs_I(BadInt3()))

self.assertEqual(UINT_MAX, getargs_I(-1))
Expand Down Expand Up @@ -290,15 +290,15 @@ def test_h(self):
self.assertEqual(99, getargs_h(Index()))
self.assertEqual(0, getargs_h(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_h, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_h(BadIndex2()))
with self.assertRaises(TypeError):
getargs_h(BadIndex2())
self.assertEqual(0, getargs_h(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_h(Int()))
self.assertEqual(0, getargs_h(IntSubclass()))
self.assertRaises(TypeError, getargs_h, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_h(BadInt2()))
with self.assertRaises(TypeError):
getargs_h(BadInt2())
self.assertEqual(0, getargs_h(BadInt3()))

self.assertRaises(OverflowError, getargs_h, SHRT_MIN-1)
Expand All @@ -316,15 +316,15 @@ def test_i(self):
self.assertEqual(99, getargs_i(Index()))
self.assertEqual(0, getargs_i(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_i, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_i(BadIndex2()))
with self.assertRaises(TypeError):
getargs_i(BadIndex2())
self.assertEqual(0, getargs_i(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_i(Int()))
self.assertEqual(0, getargs_i(IntSubclass()))
self.assertRaises(TypeError, getargs_i, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_i(BadInt2()))
with self.assertRaises(TypeError):
getargs_i(BadInt2())
self.assertEqual(0, getargs_i(BadInt3()))

self.assertRaises(OverflowError, getargs_i, INT_MIN-1)
Expand All @@ -342,15 +342,15 @@ def test_l(self):
self.assertEqual(99, getargs_l(Index()))
self.assertEqual(0, getargs_l(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_l, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_l(BadIndex2()))
with self.assertRaises(TypeError):
getargs_l(BadIndex2())
self.assertEqual(0, getargs_l(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_l(Int()))
self.assertEqual(0, getargs_l(IntSubclass()))
self.assertRaises(TypeError, getargs_l, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_l(BadInt2()))
with self.assertRaises(TypeError):
getargs_l(BadInt2())
self.assertEqual(0, getargs_l(BadInt3()))

self.assertRaises(OverflowError, getargs_l, LONG_MIN-1)
Expand All @@ -367,11 +367,12 @@ def test_n(self):
# (PY_SSIZE_T_MIN ... PY_SSIZE_T_MAX)
self.assertRaises(TypeError, getargs_n, 3.14)
self.assertEqual(99, getargs_n(Index()))
self.assertEqual(0, getargs_n(IndexIntSubclass()))
self.assertEqual(getargs_n(IndexIntSubclass()), 99)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: inconsistent arg ordering. Will fix.

self.assertRaises(TypeError, getargs_n, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_n(BadIndex2()))
self.assertEqual(0, getargs_n(BadIndex3()))
with self.assertRaises(TypeError):
getargs_n(BadIndex2())
with self.assertRaises(TypeError):
getargs_n(BadIndex3())
self.assertRaises(TypeError, getargs_n, Int())
self.assertEqual(0, getargs_n(IntSubclass()))
self.assertRaises(TypeError, getargs_n, BadInt())
Expand All @@ -397,15 +398,14 @@ def test_L(self):
self.assertEqual(99, getargs_L(Index()))
self.assertEqual(0, getargs_L(IndexIntSubclass()))
self.assertRaises(TypeError, getargs_L, BadIndex())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_L(BadIndex2()))
with self.assertRaises(TypeError):
getargs_L(BadIndex2())
self.assertEqual(0, getargs_L(BadIndex3()))
with self.assertWarns(DeprecationWarning):
self.assertEqual(99, getargs_L(Int()))
self.assertEqual(99, getargs_L(Int()))
self.assertEqual(0, getargs_L(IntSubclass()))
self.assertRaises(TypeError, getargs_L, BadInt())
with self.assertWarns(DeprecationWarning):
self.assertEqual(1, getargs_L(BadInt2()))
with self.assertRaises(TypeError):
getargs_L(BadInt2())
self.assertEqual(0, getargs_L(BadInt3()))

self.assertRaises(OverflowError, getargs_L, LLONG_MIN-1)
Expand Down
13 changes: 6 additions & 7 deletions Lib/test/test_index.py
Expand Up @@ -66,10 +66,10 @@ def __index__(self):
direct_index = my_int.__index__()
operator_index = operator.index(my_int)
self.assertEqual(direct_index, 8)
self.assertEqual(operator_index, 7)
self.assertEqual(operator_index, 8)
Copy link
Member Author

@mdickinson mdickinson Jun 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change: for a subclass of int, we take the __index__ result to be the true value instead of the integer's value.

# Both results should be of exact type int.
self.assertIs(type(direct_index), int)
#self.assertIs(type(operator_index), int)
self.assertIs(type(operator_index), int)

def test_index_returns_int_subclass(self):
class BadInt:
Expand All @@ -81,13 +81,12 @@ def __index__(self):
return True

bad_int = BadInt()
with self.assertWarns(DeprecationWarning):
n = operator.index(bad_int)
self.assertEqual(n, 1)
with self.assertRaises(TypeError):
operator.index(bad_int)

bad_int = BadInt2()
n = operator.index(bad_int)
self.assertEqual(n, 0)
with self.assertRaises(TypeError):
operator.index(bad_int)


class SeqTestCase:
Expand Down
28 changes: 10 additions & 18 deletions Lib/test/test_int.py
Expand Up @@ -488,39 +488,31 @@ def __trunc__(self):
return True

bad_int = BadIndex()
with self.assertWarns(DeprecationWarning):
with self.assertRaises(TypeError):
n = int(bad_int)
self.assertEqual(n, 1)
self.assertIs(type(n), int)

bad_int = BadIndex2()
n = int(bad_int)
self.assertEqual(n, 0)
self.assertIs(type(n), int)

bad_int = BadInt()
with self.assertWarns(DeprecationWarning):
n = int(bad_int)
self.assertEqual(n, 1)
self.assertIs(type(n), int)
with self.assertRaises(TypeError):
int(bad_int)

bad_int = BadInt2()
with self.assertWarns(DeprecationWarning):
n = int(bad_int)
self.assertEqual(n, 1)
self.assertIs(type(n), int)
with self.assertRaises(TypeError):
int(bad_int)

bad_int = TruncReturnsBadIndex()
with self.assertWarns(DeprecationWarning):
with self.assertRaises(TypeError):
n = int(bad_int)
self.assertEqual(n, 1)
self.assertIs(type(n), int)
import pdb; pdb.set_trace()
print("bob")

bad_int = TruncReturnsBadInt()
with self.assertWarns(DeprecationWarning):
n = int(bad_int)
self.assertEqual(n, 1)
self.assertIs(type(n), int)
with self.assertRaises(TypeError):
int(bad_int)

good_int = TruncReturnsIntSubclass()
n = int(good_int)
Expand Down
@@ -0,0 +1,2 @@
Remove long-deprecated allowance for ``__int__`` and ``__index__`` methods
to return subclasses of ``int``.
20 changes: 7 additions & 13 deletions Objects/abstract.c
Expand Up @@ -1269,36 +1269,30 @@ PyNumber_Index(PyObject *item)
return null_error();
}

if (PyLong_Check(item)) {
/* Fast path for the case that we already have an int. */
if (PyLong_CheckExact(item)) {
Py_INCREF(item);
return item;
}

/* In general, use __index__ */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: redundant comment. I'll remove it.

if (!PyIndex_Check(item)) {
PyErr_Format(PyExc_TypeError,
"'%.200s' object cannot be interpreted "
"as an integer", item->ob_type->tp_name);
return NULL;
}
result = item->ob_type->tp_as_number->nb_index(item);
if (!result || PyLong_CheckExact(result))
if (result == NULL || PyLong_CheckExact(result)) {
return result;
if (!PyLong_Check(result)) {
}
else {
PyErr_Format(PyExc_TypeError,
"__index__ returned non-int (type %.200s)",
result->ob_type->tp_name);
Py_DECREF(result);
return NULL;
}
/* Issue #17576: warn if 'result' not of exact type int. */
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
"__index__ returned non-int (type %.200s). "
"The ability to return an instance of a strict subclass of int "
"is deprecated, and may be removed in a future version of Python.",
result->ob_type->tp_name)) {
Py_DECREF(result);
return NULL;
}
return result;
}

/* Return an error on Overflow only if err is not NULL*/
Expand Down
31 changes: 6 additions & 25 deletions Objects/longobject.c
Expand Up @@ -163,25 +163,16 @@ _PyLong_FromNbInt(PyObject *integral)
/* Convert using the nb_int slot, which should return something
of exact type int. */
result = nb->nb_int(integral);
if (!result || PyLong_CheckExact(result))
if (!result || PyLong_CheckExact(result)) {
return result;
if (!PyLong_Check(result)) {
}
else {
PyErr_Format(PyExc_TypeError,
"__int__ returned non-int (type %.200s)",
result->ob_type->tp_name);
Py_DECREF(result);
return NULL;
}
/* Issue #17576: warn if 'result' not of exact type int. */
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
"__int__ returned non-int (type %.200s). "
"The ability to return an instance of a strict subclass of int "
"is deprecated, and may be removed in a future version of Python.",
result->ob_type->tp_name)) {
Py_DECREF(result);
return NULL;
}
return result;
}

/* Convert the given object to a PyLongObject using the nb_index or
Expand Down Expand Up @@ -216,26 +207,16 @@ _PyLong_FromNbIndexOrNbInt(PyObject *integral)
/* Convert using the nb_index slot, which should return something
of exact type int. */
result = nb->nb_index(integral);
if (!result || PyLong_CheckExact(result))
if (!result || PyLong_CheckExact(result)) {
return result;
if (!PyLong_Check(result)) {
}
else {
PyErr_Format(PyExc_TypeError,
"__index__ returned non-int (type %.200s)",
result->ob_type->tp_name);
Py_DECREF(result);
return NULL;
}
/* Issue #17576: warn if 'result' not of exact type int. */
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
"__index__ returned non-int (type %.200s). "
"The ability to return an instance of a strict subclass of int "
"is deprecated, and may be removed in a future version of Python.",
result->ob_type->tp_name))
{
Py_DECREF(result);
return NULL;
}
return result;
}

result = _PyLong_FromNbInt(integral);
Expand Down