PyNumber_Index() is not int-subclass friendly (or operator.index() docs lie)

[warsaw]

BPO	17576
Nosy	@warsaw, @brettcannon, @rhettinger, @mdickinson, @ncoghlan, @vstinner, @alex, @ethanfurman, @ericsnowcurrently, @serhiy-storchaka, @manueljacob
PRs	bpo-17576: Strict __int__ and __index__ return types; operator.index always uses __index__ #13740
Files	issue17576.patch issue17576_v2.patch issue17576_v3.patch issue17576_v4.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ethanfurman'
closed_at = None
created_at = <Date 2013-03-29.22:25:11.167>
labels = ['interpreter-core', 'type-bug', '3.9', '3.10']
title = 'PyNumber_Index() is not int-subclass friendly (or operator.index() docs lie)'
updated_at = <Date 2021-08-31.15:42:26.900>
user = 'https://github.com/warsaw'

bugs.python.org fields:

activity = <Date 2021-08-31.15:42:26.900>
actor = 'vstinner'
assignee = 'ethan.furman'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2013-03-29.22:25:11.167>
creator = 'barry'
dependencies = []
files = ['31147', '31149', '33077', '33093']
hgrepos = []
issue_num = 17576
keywords = ['patch']
message_count = 54.0
messages = ['185522', '185523', '185530', '185531', '185532', '188791', '188810', '194335', '194337', '194347', '195713', '195747', '195759', '195764', '195772', '195824', '200171', '200229', '205733', '205735', '205793', '205802', '205856', '205862', '205870', '205889', '205890', '205896', '205916', '205919', '205921', '206199', '207293', '236493', '236502', '236526', '236528', '236880', '336045', '344266', '344275', '344308', '344384', '344386', '344406', '344681', '349562', '370171', '370177', '370190', '381639', '400674', '400707', '400747']
nosy_count = 14.0
nosy_names = ['barry', 'brett.cannon', 'rhettinger', 'mark.dickinson', 'ncoghlan', 'vstinner', 'Arfrever', 'alex', 'docs@python', 'ethan.furman', 'python-dev', 'eric.snow', 'serhiy.storchaka', 'mjacob']
pr_nums = ['13740']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue17576'
versions = ['Python 3.9', 'Python 3.10']

[warsaw]

operator.index() is just a thin wrapper around PyNumber_Index(). The documentation for operator.index() claims that it is equivalent to calling obj.__index__() but for subclasses of int, this is not true. In fact, PyNumber_Index() first does (e.g. in Python 3.3) a PyLong_Check() and if that succeeds, the original object is returned *without* doing the moral equivalent in C of calling obj.__index__(). An example:

class myint(int):
 def __index__(self):
 return int(self) + 1

>>> x = myint(7)
>>> x.__index__()
8
>>> from operator import index
>>> index(x)
7

The C API documents PyNumber_Index() as: "Returns the o converted to a Python int on success or NULL with a TypeError exception raised on failure."

Because this has been the behavior of PyNumber_Index() since at least 2.7 (I didn't check farther back), this probably cannot be classified as a bug deserving to be fixed in the code for older Pythons. It might be worth fixing for Python 3.4, i.e. by moving the index check before the type check. In the meantime, this is probably a documentation bug.

The C API implies, but should be clearer that if o is an int subtype (int and long in Python 2), it is returned unchanged. The operator.index() documentation should be amended to describe this behavior for int/long subclasses.

A different alternative would be to leave PyNumber_Index() unchanged, but with the doco fix, and to augment operator.index() to do the PyIndex_Check() first, before calling PyNumber_Index(). That's a little more redundant, but would provide the documented behavior without changing the C API.

[warsaw]

You also end up with this nice bit of inconsistency:

>>> x = myint(7)
>>> from operator import index
>>> range(10)[6:x]
range(6, 7)
>>> range(10)[6:x.__index__()]
range(6, 8)
>>> range(10)[6:index(x)]
range(6, 7)
>>> 

Granted, it's insane to have __index__() return a different value like this, but in my specific use case, it's the type of object returned from operator.index() that's the problem. operator.index() returns the subclass instance while obj.__index__() returns the int.

(The use case is the IntEnum of PEP-435.)

[ericsnowcurrently]

Would it be okay to do a check on __index__ after the PyLong_Check() succeeds? Something like this:

 if (PyLong_Check(item) &&
 item->ob_type->tp_as_number->nb_index == PyLong_Type.tp_as_number->nb_index) {
 Py_INCREF(item);
 return item;
 }

This is something Nick and I were talking about at the sprints regarding fast paths in the abstract API (for mappings and sequences in our case).

[alex]

In my opinion that should use PyLong_CheckExact

[warsaw]

On Mar 30, 2013, at 12:29 AM, Eric Snow wrote:

Would it be okay to do a check on __index__ after the PyLong_Check()
succeeds? Something like this:

if (PyLong_Check(item) &&
item->ob_type->tp_as_number->nb_index == PyLong_Type.tp_as_number->nb_index) {
Py_INCREF(item);
return item;
}

This is something Nick and I were talking about at the sprints regarding fast
paths in the abstract API (for mappings and sequences in our case).

I think that would work, yes. With this extra check, overriding __index__()
in the subclass should fail this condition and fall back to the
PyIndex_Check() clause.

[vstinner]

Alex> In my opinion that should use PyLong_CheckExact

+1

[serhiy-storchaka]

if (PyLong_CheckExact(item) || (PyLong_Check(item) &&
item->ob_type->tp_as_number->nb_index == PyLong_Type.tp_as_number->nb_index))

[mdickinson]

See the related python-dev discussion started by Mark Shannon here:

http://mail.python.org/pipermail/python-dev/2013-March/125022.html

and continuing well into April here:

http://mail.python.org/pipermail/python-dev/2013-April/125042.html

The consensus that emerged from that thread seems to be that calls to operator.index and to int() should always return something of exact type int.

The attached patch:

• Raises TypeError for implicit calls to nb_int that fail to return something of exact type int. (Results of direct calls to __int__ are not checked.)

• Ensures that *all* conversions from a non-int to an int via nb_int make use of the nb_int slot, even for int subclasses. Prior to this patch, some of the PyLong_As... functions would bypass __int__ for int subclasses.

• Adds a new private _PyLong_FromNbInt function to Objects/longobject.c, so that we have a single place for performing these conversions and making type checks, and refactors existing uses of the nb_int slot to go via this function.

• Makes corresponding changes for nb_index, which should address the original bug report.

I guess this may be too dangerous a change for Python 3.4. In that case, I propose raising warnings instead of TypeErrors for Python 3.4 and turning those into TypeErrors in Python 3.5.

One other question: should direct calls to __int__ and __index__ also have their return values type-checked? That doesn't seem to happen at the moment for other magic methods (see below), so it would seem consistent to only do the type checking for interpreter-generated implicit calls to __int__ and __index__. Nick: any opinion?

>>> class A:
... def __len__(self): return "a string"
... def __bool__(self): return "another string"
... 
>>> a = A()
>>> a.__len__()
'a string'
>>> a.__bool__()
'another string'
>>> len(a)
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
TypeError: 'str' object cannot be interpreted as an integer
>>> bool(a)
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
TypeError: __bool__ should return bool, returned str