This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: numbers.Rational implements __float__ incorrectly
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Sergey.Kirpichev, mark.dickinson, paul.moore, wolma
Priority: normal Keywords: patch

Created on 2015-04-16 20:06 by wolma, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
Rational.__float__.patch wolma, 2015-04-27 13:08 review
Pull Requests
URL Status Linked Edit
PR 25619 open Sergey.Kirpichev, 2021-04-26 04:22
Messages (7)
msg241270 - (view) Author: Wolfgang Maier (wolma) * Date: 2015-04-16 20:06
numbers.Rational defines __float__ like this:

def __float__(self):
    """float(self) = self.numerator / self.denominator

    It's important that this conversion use the integer's "true"
    division rather than casting one side to float before dividing
    so that ratios of huge integers convert without overflowing.

    """
    return self.numerator / self.denominator


This assumes that division of two Integral numbers returns a float, which the numbers ABC does not enforce in any way.
IMO, the only logical assumption is that division of any two Integral or Rational numbers gives a Real, for which the ABC guarantees a __float__ method in turn.
So I think Rational.__float__ should

return float(self.numerator / self.denominator)
msg241340 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2015-04-17 16:07
What happens if `self.numerator / self.denominator` returns another `Rational` instance?  If I create `MyRational` and `MyInteger` classes, such that the quotient of two `MyInteger` instances is a `MyRational` instance and the numerator and denominator of a `MyRational` instance are both `MyInteger` instances, this suggestion would lead to an infinite recursion.  OTOH, I agree that *something* needs to be fixed here, since having `__float__` return a `MyRational` instance is definitely wrong.
msg241636 - (view) Author: Wolfgang Maier (wolma) * Date: 2015-04-20 12:38
Good point.

If the numbers ABC guaranteed numerator and denominator to be Integral numbers, this could be solved by:

return float(int(self.numerator) / int(self.denominator))

but since both could be Rationals again that does not seem to be an option either.
What could be done is trying to multiply out the numerator and denominator pair until both *are* Integrals, like:

num = self.numerator
den = self.denominator
while not (isinstance(num, Integral) and isinstance(den, Integral)):
    num = num.numerator * den.denominator
    den = den.numerator * num.denominator
return float(int(num) / int(den))

Clearly that's more complicated, but, more importantly, has the disadvantage that the loop will run forever if the final numerator or denominator is not registered correctly in the numeric tower.
So some kind of check for this situation would be required, but I do not have an idea right now what that should look like.
msg241637 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2015-04-20 12:50
Is it not reasonable to simply say that implementations of numbers.Rational which allow the numerator and denominator to have types for which true division doesn't return a float, have to provide their own implementation of __float__()?

It's certainly less convenient, and probably surprising for users, but the alternative is trying to work around broken integer types - after all numbers.Complex.__truediv__ says "Should promote to float when necessary" in the docstring, which to me says that a type where a/b doesn't return a float doesn't conform to the numeric tower.
msg241638 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2015-04-20 12:55
Alternatively, return int(self.numerator) / int(self.denominator). After all, a fraction whose numerator can't be represented as a Python (unlimited precision) integer is a pretty odd sort of fraction...
msg241639 - (view) Author: Wolfgang Maier (wolma) * Date: 2015-04-20 13:05
> Is it not reasonable to simply say that implementations of numbers.Rational which allow the numerator and denominator to have types for which true division doesn't return a float, have to provide their own implementation of __float__()?

Unfortunately, the Rational type cannot always know what its numerator or denominator supports.
Look at fractions.Fraction: its only requirement for numerator and denominator is that they both should be Rational instances.
So a hypothetical MyInt like Mark describes it could be perfectly acceptable for Fraction except that it would fail to convert it to float.
 
> It's certainly less convenient, and probably surprising for users, but the alternative is trying to work around broken integer types - after all numbers.Complex.__truediv__ says "Should promote to float when necessary" in the docstring, which to me says that a type where a/b doesn't return a float doesn't conform to the numeric tower.
> 

I do read this docstring differently. To me, it means should promote to float if there is no other way to express the result (like when your custom type system does not define its own Float type).
It would, in fact, be really bad for custom type implementors if they would be forced to leave their type system when doing divisions.

> Alternatively, return int(self.numerator) / int(self.denominator). After all, a fraction whose numerator can't be represented as a Python (unlimited precision) integer is a pretty odd sort of fraction...

The problem here is that self.numerator and/or self.denominator could both be Rational instances again, which are not expressible as a Python integer and, thus, are not enforced to provide __int__ by the numbers ABC.
msg242113 - (view) Author: Wolfgang Maier (wolma) * Date: 2015-04-27 13:08
After considering this for a while, I think:

return float(self.numerator / self.denominator)

is the best solution:

* it is simple and works reasonably well as a default

* it fixes Rational.__float__ for cases, in which numerator / denominator returns a custom Real instance

* in the problematic scenario brought up by Mark, in which truediv of the numerator and denominator returns another Rational creating a potentially infinite recursion, a RuntimeError will be raised when the maximum recursion limit is reached. This is not an unheard of error to run into while trying to implement a custom numeric type and will provide reasonable (though not ideal) information about the problem.

* the workaround for the above is obvious: it is ok for self.numerator / self.denominator to return a Rational instance, but its type should overwrite Rational.__float__ to break the recursion. This could get documented in the docstring of Rational.__float__.

I've attached a tentative patch.
History
Date User Action Args
2022-04-11 14:58:15adminsetgithub: 68163
2021-04-26 04:22:28Sergey.Kirpichevsetstage: patch review
pull_requests: + pull_request24321
2021-04-23 08:42:05Sergey.Kirpichevsetnosy: + Sergey.Kirpichev
2015-04-27 13:08:22wolmasetfiles: + Rational.__float__.patch
keywords: + patch
messages: + msg242113
2015-04-20 13:05:02wolmasetmessages: + msg241639
2015-04-20 12:55:46paul.mooresetmessages: + msg241638
2015-04-20 12:50:26paul.mooresetnosy: + paul.moore
messages: + msg241637
2015-04-20 12:38:35wolmasetmessages: + msg241636
2015-04-17 16:07:59mark.dickinsonsetmessages: + msg241340
2015-04-16 22:17:45r.david.murraysetnosy: + mark.dickinson
2015-04-16 20:06:57wolmacreate