int() and math.trunc don't accept objects that only define __index__

[ncoghlan]

BPO	33039
Nosy	@mdickinson, @ncoghlan, @serhiy-storchaka, @appeltel, @csabella, @remilapeyre, @tirkarthi
PRs	bpo-20092: Make __int__ defaults to __index__ #13106
Superseder	bpo-20092: type() constructor should bind int to index when index is defined and int is not

^{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 = None
closed_at = <Date 2019-05-06.14:29:29.151>
created_at = <Date 2018-03-10.08:57:10.957>
labels = ['3.8', 'type-feature', '3.7', 'docs']
title = "int() and math.trunc don't accept objects that only define __index__"
updated_at = <Date 2019-05-06.14:29:29.150>
user = 'https://github.com/ncoghlan'

bugs.python.org fields:

activity = <Date 2019-05-06.14:29:29.150>
actor = 'serhiy.storchaka'
assignee = 'docs@python'
closed = True
closed_date = <Date 2019-05-06.14:29:29.151>
closer = 'serhiy.storchaka'
components = ['Documentation']
creation = <Date 2018-03-10.08:57:10.957>
creator = 'ncoghlan'
dependencies = []
files = []
hgrepos = []
issue_num = 33039
keywords = ['patch']
message_count = 10.0
messages = ['313515', '313522', '332819', '333546', '336044', '336201', '336234', '340453', '341505', '341508']
nosy_count = 8.0
nosy_names = ['mark.dickinson', 'ncoghlan', 'docs@python', 'serhiy.storchaka', 'Eric Appelt', 'cheryl.sabella', 'remi.lapeyre', 'xtreak']
pr_nums = ['13106']
priority = 'normal'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '20092'
type = 'enhancement'
url = 'https://bugs.python.org/issue33039'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[ncoghlan]

(Note: I haven't categorised this yet, as I'm not sure how it *should* be categorised)

Back when the __index__/nb_index slot was added, the focus was on allowing 3rd party integer types to be used in places where potentially lossy conversion with __int__/nb_int *wasn't* permitted.

However, this has led to an anomaly where the lossless conversion method *isn't* tried implicitly for the potentially lossy int() and math.trunc() calls, but is tried automatically in other contexts:

>>> import math
>>> class MyInt:
...     def __index__(self):
...         return 42
... 
>>> int(MyInt())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: int() argument must be a string, a bytes-like object or a number, not 'MyInt'
>>> math.trunc(MyInt())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: type MyInt doesn't define __trunc__ method
>>> hex(MyInt())
'0x2a'
>>> len("a" * MyInt())
42

Supporting int() requires also setting __int__:

>>> MyInt.__int__ = MyInt.__index__
>>> int(MyInt())
42

Supporting math.trunc() requires also setting __trunc__:

>>> MyInt.__trunc__ = MyInt.__index__
>>> math.trunc(MyInt())
42

(This anomaly was noticed by Eric Appelt while updating the int() docs to cover the fallback to trying __trunc__ when __int__ isn't defined: #6022 (comment))

[ncoghlan]

Marking this as a documentation enhancement request for now, but I think we should also consider changing the type creation behaviour in 3.8 to implicitly add __int__ and __trunc__ definitions when __index__ is defined, but they aren't.

That way, no behaviour will change for classes that explicitly define __int__ or __trunc__, but classes that only define __index__ without defining the other methods will behave more intuitively.

[remilapeyre]

I think we should also consider changing the type creation behaviour in 3.8

@ncoghlan is this what's being done in PyTypeReady?

[ncoghlan]

@rémi Aye, filling out derived slots is one of the things PyType_Ready does.

This would need a discussion on python-dev before going ahead and doing it though, as the closest equivalent we have to this right now is the "negative" derivation, where overriding __eq__ without overriding __hash__ implicitly marks the derived class as unhashable (look for "type->tp_hash = PyObject_HashNotImplemented;").

[serhiy-storchaka]

See also bpo-33039.

[serhiy-storchaka]

Is not this a duplicate of bpo-20092?

[remilapeyre]

Yes it is. Thanks for finding that @serhiy.

Since nobody objected to the change on the mailing list and people seem to agree in bpo-20092:

[R. David Murray]
To summarize for anyone like me who didn't follow that issue: __index__ means the object can be losslessly converted to an int (is a true int), while __int__ may be an approximate conversion.  Thus it makes sense for an object to have an __int__ but not __index__, but vice-versa does not make sense.

I will post my patch tonight.

[csabella]

Rémi,

Are you still working on the patch for this? Thanks!

[remilapeyre]

Hi Cheryl,

thanks for the ping.

I wasn't sure my patch was correct but reading typeobject.c:add_operators(), it is actually more straight-forward than I thought.

Serhiy Storchaka: This is indeed a duplicate of bpo-20092. I believe the solution proposed by Nick Coghlan is better than the one of Amitava Bhattacharyya, "adding a call to nb_index (if that slot exists) in _PyLong_FromNbInt" though.

One thing to note regarding the proposed patch: the following stops to work and raises a RecursionError since __index__ == __int__:

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

I changed test_int_subclass_with_index() as int(self) + 1 is the same thing as self + 1 for int subclasses. I don't think this sort of code should appear in the wild but if you think it is important not to break compatibility here, I think I could check for number subclasses before overriding index.

[serhiy-storchaka]

Then let to continue the discussion on the older issue which has larger discussion.