bpo-23975: Correct conversion of Rational’s to float#25619
bpo-23975: Correct conversion of Rational’s to float#25619mdickinson merged 10 commits intopython:mainfrom skirpichev:fix-23975
Conversation
|
This PR is stale because it has been open for 30 days with no activity. |
Also document that numerator/denominator properties are instances of the Integral.
|
@mdickinson, you are listed as an expert for the fractions module, could you review this little PR? (This, probably, lacks a test.) |
|
@skirpichev Seems like a reasonable approach to me. But as you say, it needs a test before it can go in. (There should be at least something in the test suite that fails if those |
Ok, lets see how it looks. |
|
Hmm, unfortunately, there is a silent type-cast. I can adjust the test, but may be it does make sense to add diff --git a/Lib/fractions.py b/Lib/fractions.py
index f9ac882ec0..4a153ee8e3 100644
--- a/Lib/fractions.py
+++ b/Lib/fractions.py
@@ -143,6 +143,10 @@ def __new__(cls, numerator=0, denominator=None, *, _normalize=True):
elif type(numerator) is int is type(denominator):
pass # *very* normal case
+ elif (isinstance(numerator, numbers.Integral) and
+ isinstance(denominator, numbers.Integral)):
+ pass
+
elif (isinstance(numerator, numbers.Rational) and
isinstance(denominator, numbers.Rational)):
numerator, denominator = (... or I can just "fix" type of private |
|
@mdickinson, let me know if you have some objections for added tests (maybe Lib/test/test_abstract_numbers.py is the right place to this?). I've realized that there almost no tests for default methods of the numbers.py (#32022 address this issue, partially). |
|
The fix looks simple but where does the current implementation fail? Can you provide a test case such that the current fails but the after the fix the test succeeds? |
|
@MaxwellDupre, the added test should be such one. At least it was a failing test for the old main branch (~Apr 2021). |
|
Thanks @skirpichev for the PR, and @mdickinson for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
|
GH-96556 is a backport of this pull request to the 3.11 branch. |
…thonGH-25619) * pythongh-68163: Correct conversion of Rational instances to float Also document that numerator/denominator properties are instances of Integral. Co-authored-by: Mark Dickinson <dickinsm@gmail.com> (cherry picked from commit 8464b75) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
|
GH-96557 is a backport of this pull request to the 3.10 branch. |
…thonGH-25619) * pythongh-68163: Correct conversion of Rational instances to float Also document that numerator/denominator properties are instances of Integral. Co-authored-by: Mark Dickinson <dickinsm@gmail.com> (cherry picked from commit 8464b75) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Also document that numerator/denominator properties are instances of the Integral.
This solution was suggested by Paul Moore.
https://bugs.python.org/issue23975