bpo-44547: Make Fractions objects instances of typing.SupportsInt #27851
Conversation
What if numerator // denominator
is an Integral, but not int
(for example if numerator
and denominator
are GMP integers)?
The int constructor implicitly calls index()
for the result of __trunc__()
, but __int__()
should return an exact int
.
Aargh; good point. I'll fix, and add a test for that case. |
Hmm. The fix is easy; the test less so. The most obvious way to test is to actually create a non-int-subclass that implements |
I think that we can use an int subclass for simple testing: class I(int):
def __floordiv__(self, other):
return I(int(self) // other) |
Yes, that's probably good enough. Updated. |
Lib/fractions.py
Outdated
"""trunc(a)""" | ||
"""math.trunc(a)""" | ||
# Note: this differs from __int__ - __int__ must return an int, | ||
# while __trunc__ may return a non-int numbers.Integral object |
I think that __trunc__()
can return non-Integral (for example a NumPy array), it will be just incompatible with int()
constructor.
Yes, true. What do you think about just deleting the second line? (Or maybe simply deleting the whole comment.)
Lib/fractions.py
Outdated
def __int__(a): | ||
"""int(a)""" | ||
if a._numerator < 0: | ||
return int(-(-a._numerator // a._denominator)) |
Would not be better to use operator.index()
?
I'm not sure why; int
seems more natural here, given that we're implementing __int__
, and it seems a little bit more obvious to me that int
must return something of type int
(compared to __index__
). I think it doesn't really matter all that much - anything implementing Integral
and returning different results from __int__
and __index__
is asking for trouble.
Also, in numbers.Integral
, __int__
is the more fundamental method: __index__
is implemented in terms of __int__
by default.
What is the performance impact of this change on int()? |
No idea, but I wouldn't expect it to be significant in either direction. I can do some testing, but performance doesn't seem like it ought to be a major concern here. |
Hmm; I was wrong. I do see a significant difference in casual timings, apparently arising from the slowness of the 'int' call. On this branch:
On the main branch:
Using
I get something that's consistently faster than either of the above:
|
Just wondering, what is the performance if remove the index call? |
Should this change be backported? If no, it should be documented as a new feature (versionchanged and What's New). |
Definitely an enhancement so no backports. |
This PR adds an
__int__
method tofractions.Fraction
, so that anisinstance(some_fraction, typing.SupportsInt)
check passes.https://bugs.python.org/issue44547
The text was updated successfully, but these errors were encountered: