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

Conversation

@mdickinson
Copy link
Member

commented Jun 2, 2019

This PR:

  • turns the DeprecationWarnings introduced by @serhiy-storchaka in Python 3.4 for non-integer return values from __int__ and __index__ into TypeErrors
  • makes operator.index and PyNumber_Index always give the same return value as __index__. This fixes the inconsistency reported by @warsaw in bpo-17576.

Still working on doc updates and what's-new entries, but making the PR now so that others can review.

https://bugs.python.org/issue17576

@mdickinson

This comment has been minimized.

Copy link
Member Author

commented Jun 2, 2019

This PR should also allow simplifications to many parts of the codebase that currently need to check what type they got back from a PyNumber_Index call.

@@ -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)

This comment has been minimized.

Copy link
@mdickinson

mdickinson Jun 2, 2019

Author Member

Nitpick: inconsistent arg ordering. Will fix.

@@ -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)

This comment has been minimized.

Copy link
@mdickinson

mdickinson Jun 2, 2019

Author Member

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.

Py_INCREF(item);
return item;
}

/* In general, use __index__ */

This comment has been minimized.

Copy link
@mdickinson

mdickinson Jun 2, 2019

Author Member

Nitpick: redundant comment. I'll remove it.

@mdickinson

This comment has been minimized.

Copy link
Member Author

commented Jun 2, 2019

The test failure appears to be due to test_asyncio altering the test environment. It seems unlikely that it's related to the changes in this PR (but anything's possible).

@serhiy-storchaka

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

Some of these warnings were added in 3.8. We cannot turn them into TypeError in the same version.

@tirkarthi

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

The test failure appears to be due to test_asyncio altering the test environment. It seems unlikely that it's related to the changes in this PR (but anything's possible).

Could you please try rebasing on master to see if it helps. One of the assertion errors in test_asyncio was fixed with #13754 .

@mdickinson

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

Some of these warnings were added in 3.8.

Are you sure? Which ones? The PR removes 3 DeprecationWarning blocks: one in abstract.c and two in longobject.c, and it looks to me as though all three were introduced in 31a6554

@serhiy-storchaka

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

The warning added in bpo-36048.

@mdickinson

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

The warning added in bpo-36048.

Ah yes, right.

Good point. Closing.

@mdickinson mdickinson closed this Jun 3, 2019

@mdickinson mdickinson deleted the mdickinson:make-int-and-index-return-types-strict branch Jun 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.