Skip to content

Commit

Permalink
gh-101773: Optimize creation of Fractions in private methods (#101780)
Browse files Browse the repository at this point in the history
This PR adds a private `Fraction._from_coprime_ints` classmethod for internal creations of `Fraction` objects, replacing the use of `_normalize=False` in the existing constructor. This speeds up creation of `Fraction` objects arising from calculations. The `_normalize` argument to the `Fraction` constructor has been removed.

Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
  • Loading branch information
3 people authored Feb 27, 2023
1 parent bb0cf8f commit 4f3786b
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 34 deletions.
79 changes: 46 additions & 33 deletions Lib/fractions.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class Fraction(numbers.Rational):
__slots__ = ('_numerator', '_denominator')

# We're immutable, so use __new__ not __init__
def __new__(cls, numerator=0, denominator=None, *, _normalize=True):
def __new__(cls, numerator=0, denominator=None):
"""Constructs a Rational.
Takes a string like '3/2' or '1.5', another Rational instance, a
Expand Down Expand Up @@ -279,12 +279,11 @@ def __new__(cls, numerator=0, denominator=None, *, _normalize=True):

if denominator == 0:
raise ZeroDivisionError('Fraction(%s, 0)' % numerator)
if _normalize:
g = math.gcd(numerator, denominator)
if denominator < 0:
g = -g
numerator //= g
denominator //= g
g = math.gcd(numerator, denominator)
if denominator < 0:
g = -g
numerator //= g
denominator //= g
self._numerator = numerator
self._denominator = denominator
return self
Expand All @@ -301,7 +300,7 @@ def from_float(cls, f):
elif not isinstance(f, float):
raise TypeError("%s.from_float() only takes floats, not %r (%s)" %
(cls.__name__, f, type(f).__name__))
return cls(*f.as_integer_ratio())
return cls._from_coprime_ints(*f.as_integer_ratio())

@classmethod
def from_decimal(cls, dec):
Expand All @@ -313,7 +312,19 @@ def from_decimal(cls, dec):
raise TypeError(
"%s.from_decimal() only takes Decimals, not %r (%s)" %
(cls.__name__, dec, type(dec).__name__))
return cls(*dec.as_integer_ratio())
return cls._from_coprime_ints(*dec.as_integer_ratio())

@classmethod
def _from_coprime_ints(cls, numerator, denominator, /):
"""Convert a pair of ints to a rational number, for internal use.
The ratio of integers should be in lowest terms and the denominator
should be positive.
"""
obj = super(Fraction, cls).__new__(cls)
obj._numerator = numerator
obj._denominator = denominator
return obj

def is_integer(self):
"""Return True if the Fraction is an integer."""
Expand Down Expand Up @@ -380,9 +391,9 @@ def limit_denominator(self, max_denominator=1000000):
# the distance from p1/q1 to self is d/(q1*self._denominator). So we
# need to compare 2*(q0+k*q1) with self._denominator/d.
if 2*d*(q0+k*q1) <= self._denominator:
return Fraction(p1, q1, _normalize=False)
return Fraction._from_coprime_ints(p1, q1)
else:
return Fraction(p0+k*p1, q0+k*q1, _normalize=False)
return Fraction._from_coprime_ints(p0+k*p1, q0+k*q1)

@property
def numerator(a):
Expand Down Expand Up @@ -703,13 +714,13 @@ def _add(a, b):
nb, db = b._numerator, b._denominator
g = math.gcd(da, db)
if g == 1:
return Fraction(na * db + da * nb, da * db, _normalize=False)
return Fraction._from_coprime_ints(na * db + da * nb, da * db)
s = da // g
t = na * (db // g) + nb * s
g2 = math.gcd(t, g)
if g2 == 1:
return Fraction(t, s * db, _normalize=False)
return Fraction(t // g2, s * (db // g2), _normalize=False)
return Fraction._from_coprime_ints(t, s * db)
return Fraction._from_coprime_ints(t // g2, s * (db // g2))

__add__, __radd__ = _operator_fallbacks(_add, operator.add)

Expand All @@ -719,13 +730,13 @@ def _sub(a, b):
nb, db = b._numerator, b._denominator
g = math.gcd(da, db)
if g == 1:
return Fraction(na * db - da * nb, da * db, _normalize=False)
return Fraction._from_coprime_ints(na * db - da * nb, da * db)
s = da // g
t = na * (db // g) - nb * s
g2 = math.gcd(t, g)
if g2 == 1:
return Fraction(t, s * db, _normalize=False)
return Fraction(t // g2, s * (db // g2), _normalize=False)
return Fraction._from_coprime_ints(t, s * db)
return Fraction._from_coprime_ints(t // g2, s * (db // g2))

__sub__, __rsub__ = _operator_fallbacks(_sub, operator.sub)

Expand All @@ -741,15 +752,17 @@ def _mul(a, b):
if g2 > 1:
nb //= g2
da //= g2
return Fraction(na * nb, db * da, _normalize=False)
return Fraction._from_coprime_ints(na * nb, db * da)

__mul__, __rmul__ = _operator_fallbacks(_mul, operator.mul)

def _div(a, b):
"""a / b"""
# Same as _mul(), with inversed b.
na, da = a._numerator, a._denominator
nb, db = b._numerator, b._denominator
if nb == 0:
raise ZeroDivisionError('Fraction(%s, 0)' % db)
na, da = a._numerator, a._denominator
g1 = math.gcd(na, nb)
if g1 > 1:
na //= g1
Expand All @@ -761,7 +774,7 @@ def _div(a, b):
n, d = na * db, nb * da
if d < 0:
n, d = -n, -d
return Fraction(n, d, _normalize=False)
return Fraction._from_coprime_ints(n, d)

__truediv__, __rtruediv__ = _operator_fallbacks(_div, operator.truediv)

Expand Down Expand Up @@ -798,17 +811,17 @@ def __pow__(a, b):
if b.denominator == 1:
power = b.numerator
if power >= 0:
return Fraction(a._numerator ** power,
a._denominator ** power,
_normalize=False)
elif a._numerator >= 0:
return Fraction(a._denominator ** -power,
a._numerator ** -power,
_normalize=False)
return Fraction._from_coprime_ints(a._numerator ** power,
a._denominator ** power)
elif a._numerator > 0:
return Fraction._from_coprime_ints(a._denominator ** -power,
a._numerator ** -power)
elif a._numerator == 0:
raise ZeroDivisionError('Fraction(%s, 0)' %
a._denominator ** -power)
else:
return Fraction((-a._denominator) ** -power,
(-a._numerator) ** -power,
_normalize=False)
return Fraction._from_coprime_ints((-a._denominator) ** -power,
(-a._numerator) ** -power)
else:
# A fractional power will generally produce an
# irrational number.
Expand All @@ -832,15 +845,15 @@ def __rpow__(b, a):

def __pos__(a):
"""+a: Coerces a subclass instance to Fraction"""
return Fraction(a._numerator, a._denominator, _normalize=False)
return Fraction._from_coprime_ints(a._numerator, a._denominator)

def __neg__(a):
"""-a"""
return Fraction(-a._numerator, a._denominator, _normalize=False)
return Fraction._from_coprime_ints(-a._numerator, a._denominator)

def __abs__(a):
"""abs(a)"""
return Fraction(abs(a._numerator), a._denominator, _normalize=False)
return Fraction._from_coprime_ints(abs(a._numerator), a._denominator)

def __int__(a, _index=operator.index):
"""int(a)"""
Expand Down
1 change: 1 addition & 0 deletions Lib/test/test_fractions.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ def testArithmetic(self):
self.assertEqual(F(5, 6), F(2, 3) * F(5, 4))
self.assertEqual(F(1, 4), F(1, 10) / F(2, 5))
self.assertEqual(F(-15, 8), F(3, 4) / F(-2, 5))
self.assertRaises(ZeroDivisionError, operator.truediv, F(1), F(0))
self.assertTypedEquals(2, F(9, 10) // F(2, 5))
self.assertTypedEquals(10**23, F(10**23, 1) // F(1))
self.assertEqual(F(5, 6), F(7, 3) % F(3, 2))
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_numeric_tower.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def test_fractions(self):
# The numbers ABC doesn't enforce that the "true" division
# of integers produces a float. This tests that the
# Rational.__float__() method has required type conversions.
x = F(DummyIntegral(1), DummyIntegral(2), _normalize=False)
x = F._from_coprime_ints(DummyIntegral(1), DummyIntegral(2))
self.assertRaises(TypeError, lambda: x.numerator/x.denominator)
self.assertEqual(float(x), 0.5)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Optimize :class:`fractions.Fraction` for small components. The private argument
``_normalize`` of the :class:`fractions.Fraction` constructor has been removed.

0 comments on commit 4f3786b

Please sign in to comment.