Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-101773: Optimize creation of Fractions in private methods #101780

Merged
merged 13 commits into from
Feb 27, 2023
Merged
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):
skirpichev marked this conversation as resolved.
Show resolved Hide resolved
"""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)
skirpichev marked this conversation as resolved.
Show resolved Hide resolved
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)' %
skirpichev marked this conversation as resolved.
Show resolved Hide resolved
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.