Skip to content
31 changes: 16 additions & 15 deletions Lib/fractions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@
_PyHASH_INF = sys.hash_info.inf

_RATIONAL_FORMAT = re.compile(r"""
\A\s* # optional whitespace at the start, then
(?P<sign>[-+]?) # an optional sign, then
(?=\d|\.\d) # lookahead for digit or .digit
(?P<num>\d*) # numerator (possibly empty)
(?: # followed by
(?:/(?P<denom>\d+))? # an optional denominator
| # or
(?:\.(?P<decimal>\d*))? # an optional fractional part
(?:E(?P<exp>[-+]?\d+))? # and optional exponent
\A\s* # optional whitespace at the start,
(?P<sign>[-+]?) # an optional sign, then
(?=\d|\.\d) # lookahead for digit or .digit
(?P<num>((\d+_?)*\d|\d*)) # numerator (possibly empty)
(?: # followed by
(?:/(?P<den>(\d+_?)*\d+))? # an optional denominator
| # or
(?:\.(?P<decimal>((\d+_?)*\d|\d*)))? # an optional fractional part
(?:E(?P<exp>[-+]?(\d+_?)*\d+))? # and optional exponent
)
\s*\Z # and optional whitespace to finish
\s*\Z # and optional whitespace to finish
""", re.VERBOSE | re.IGNORECASE)


Expand Down Expand Up @@ -115,15 +115,16 @@ def __new__(cls, numerator=0, denominator=None, *, _normalize=True):
raise ValueError('Invalid literal for Fraction: %r' %
numerator)
numerator = int(m.group('num') or '0')
denom = m.group('denom')
if denom:
denominator = int(denom)
denominator = m.group('den')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest reverting this set of changes; IIUC they're not needed for this PR, and I don't see any particular advantage to the rename. (In fact, I prefer the version where we don't have the same local name referring to both a string and an int.)

Copy link
Contributor Author

@skirpichev skirpichev May 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any particular advantage to the rename.

Naming of patterns seems to be more consistent in this case: num/den vs num/denom.

But you are right, this is not related to the PR. I'll revert.

I prefer the version where we don't have the same local name referring to both a string and an int

I don't think there are. We have den/denom for strings and denominator - which is an integer.

This patch may be better:

diff --git a/Lib/fractions.py b/Lib/fractions.py
index 180cd94c28..1268b6bd27 100644
--- a/Lib/fractions.py
+++ b/Lib/fractions.py
@@ -26,7 +26,7 @@
     (?=\d|\.\d)                           # lookahead for digit or .digit
     (?P<num>\d*|\d+(_\d+)*)               # numerator (possibly empty)
     (?:                                   # followed by
-       (?:/(?P<denom>\d+(_\d+)*))?        # an optional denominator
+       (?:/(?P<den>\d+(_\d+)*))?          # an optional denominator
     |                                     # or
        (?:\.(?P<decimal>d*|\d+(_\d+)*))?  # an optional fractional part
        (?:E(?P<exp>[-+]?\d+(_\d+)*))?     # and optional exponent
@@ -115,9 +115,9 @@ def __new__(cls, numerator=0, denominator=None, *, _normalize=True):
                     raise ValueError('Invalid literal for Fraction: %r' %
                                      numerator)
                 numerator = int(m.group('num') or '0')
-                denom = m.group('denom')
-                if denom:
-                    denominator = int(denom)
+                den = m.group('den')
+                if den:
+                    denominator = int(den)
                 else:
                     denominator = 1
                     decimal = m.group('decimal')

if denominator:
denominator = int(denominator)
else:
denominator = 1
decimal = m.group('decimal')
if decimal:
scale = 10**len(decimal)
numerator = numerator * scale + int(decimal)
decimal = int(decimal)
scale = 10**len(str(decimal))
numerator = numerator * scale + decimal
denominator *= scale
exp = m.group('exp')
if exp:
Expand Down
58 changes: 58 additions & 0 deletions Lib/test/test_fractions.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ def testFromString(self):
self.assertEqual((-12300, 1), _components(F("-1.23e4")))
self.assertEqual((0, 1), _components(F(" .0e+0\t")))
self.assertEqual((0, 1), _components(F("-0.000e0")))
self.assertEqual((123, 1), _components(F("1_2_3")))
self.assertEqual((41, 107), _components(F("1_2_3/3_2_1")))
self.assertEqual((6283, 2000), _components(F("3.14_15")))
self.assertEqual((6283, 2*10**13), _components(F("3.14_15e-1_0")))

self.assertRaisesMessage(
ZeroDivisionError, "Fraction(3, 0)",
Expand Down Expand Up @@ -210,6 +214,60 @@ def testFromString(self):
# Allow 3. and .3, but not .
ValueError, "Invalid literal for Fraction: '.'",
F, ".")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '_'",
F, "_")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1_'",
F, "1_")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '_1'",
F, "_1")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1__2'",
F, "1__2")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '/_'",
F, "/_")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1_/'",
F, "1_/")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '_1/'",
F, "_1/")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1__2/'",
F, "1__2/")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1/_'",
F, "1/_")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1/1_'",
F, "1/1_")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1/_1'",
F, "1/_1")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1/1__2'",
F, "1/1__2")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1._111'",
F, "1._111")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1.1__1'",
F, "1.1__1")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1.1_'",
F, "1.1_")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1.1e+_1'",
F, "1.1e+_1")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1.1e+1_'",
F, "1.1e+1_")
self.assertRaisesMessage(
ValueError, "Invalid literal for Fraction: '1.1e+1__1'",
F, "1.1e+1__1")

def testImmutable(self):
r = F(7, 3)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support PEP 515 for Fraction's initialization from string.