Remove deprecated fractions.gcd()

[vstinner]

BPO	39350
Nosy	@mdickinson, @vstinner, @hroncok
PRs	bpo-39350: Remove deprecated fractions.gcd() #18021 bpo-39350: Fix regression introduced when fractions.gcd was removed #18309 bpo-39350: Fix fractions for int subclasses #18375

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-02-07.22:44:10.592>
created_at = <Date 2020-01-16.08:13:44.171>
labels = ['library', '3.9']
title = 'Remove deprecated fractions.gcd()'
updated_at = <Date 2020-02-07.22:44:10.592>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2020-02-07.22:44:10.592>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-02-07.22:44:10.592>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2020-01-16.08:13:44.171>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 39350
keywords = ['patch']
message_count = 12.0
messages = ['360095', '360111', '361103', '361104', '361111', '361126', '361225', '361481', '361483', '361485', '361612', '361613']
nosy_count = 3.0
nosy_names = ['mark.dickinson', 'vstinner', 'hroncok']
pr_nums = ['18021', '18309', '18375']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39350'
versions = ['Python 3.9']

[vstinner]

bpo-22486 added math.gcd() and deprecated fractions.gcd() in Python 3.5: commit 48e47aa.

The function was deprecated during 4 cycles (3.5, 3.6, 3.7, 3.8): I propose attached PR to remove it.

[vstinner]

New changeset 4691a2f by Victor Stinner in branch 'master':
bpo-39350: Remove deprecated fractions.gcd() (GH-18021)
4691a2f

[hroncok]

I believe there is a regression here.

numpy fails to build with Python 3.9.0a3.

________________________ TestMatmul.test_matmul_object _________________________
self = <numpy.core.tests.test_multiarray.TestMatmul object at 0x7ff9bc008ca0>
def test_matmul_object(self):
import fractions

    f = np.vectorize(fractions.Fraction)
    def random_ints():
        return np.random.randint(1, 1000, size=(10, 3, 3))

  M1 = f(random_ints(), random_ints())

f = <numpy.vectorize object at 0x7ff9bc6751f0>
fractions = <module 'fractions' from '/usr/lib64/python3.9/fractions.py'>
random_ints = <function TestMatmul.test_matmul_object.<locals>.random_ints at 0x7ff9bcc2d700>
self = <numpy.core.tests.test_multiarray.TestMatmul object at 0x7ff9bc008ca0>
numpy/core/tests/test_multiarray.py:6259:

---

numpy/lib/function_base.py:2091: in __call__
return self._vectorize_call(func=func, args=vargs)
numpy/lib/function_base.py:2161: in _vectorize_call
ufunc, otypes = self._get_ufunc_and_otypes(func=func, args=args)
numpy/lib/function_base.py:2121: in _get_ufunc_and_otypes
outputs = func(*inputs)

---

cls = <class 'fractions.Fraction'>, numerator = 483, denominator = 809
def __new__(cls, numerator=0, denominator=None, *, _normalize=True):
"""Constructs a Rational.

    Takes a string like '3/2' or '1.5', another Rational instance, a
    numerator/denominator pair, or a float.

    Examples
    \--------

    
        >>> Fraction(10, -8)
        Fraction(-5, 4)
        >>> Fraction(Fraction(1, 7), 5)
        Fraction(1, 35)
        >>> Fraction(Fraction(1, 7), Fraction(2, 3))
        Fraction(3, 14)
        >>> Fraction('314')
        Fraction(314, 1)
        >>> Fraction('-35/4')
        Fraction(-35, 4)
        >>> Fraction('3.1415') # conversion from numeric string
        Fraction(6283, 2000)
        >>> Fraction('-47e-2') # string may include a decimal exponent
        Fraction(-47, 100)
        >>> Fraction(1.47)  # direct construction from float (exact conversion)
        Fraction(6620291452234629, 4503599627370496)
        >>> Fraction(2.25)
        Fraction(9, 4)
        >>> Fraction(Decimal('1.47'))
        Fraction(147, 100)
    
        """
        self = super(Fraction, cls).__new__(cls)
    
        if denominator is None:
            if type(numerator) is int:
                self._numerator = numerator
                self._denominator = 1
                return self
    
            elif isinstance(numerator, numbers.Rational):
                self._numerator = numerator.numerator
                self._denominator = numerator.denominator
                return self
    
            elif isinstance(numerator, (float, Decimal)):
                # Exact conversion
                self._numerator, self._denominator = numerator.as_integer_ratio()
                return self
    
            elif isinstance(numerator, str):
                # Handle construction from strings.
                m = _RATIONAL_FORMAT.match(numerator)
                if m is None:
                    raise ValueError('Invalid literal for Fraction: %r' %
                                     numerator)
                numerator = int(m.group('num') or '0')
                denom = m.group('denom')
                if denom:
                    denominator = int(denom)
                else:
                    denominator = 1
                    decimal = m.group('decimal')
                    if decimal:
                        scale = 10**len(decimal)
                        numerator = numerator * scale + int(decimal)
                        denominator *= scale
                    exp = m.group('exp')
                    if exp:
                        exp = int(exp)
                        if exp >= 0:
                            numerator *= 10**exp
                        else:
                            denominator *= 10**-exp
                if m.group('sign') == '-':
                    numerator = -numerator
    
            else:
                raise TypeError("argument should be a string "
                                "or a Rational instance")
    
        elif type(numerator) is int is type(denominator):
            pass # *very* normal case
    
        elif (isinstance(numerator, numbers.Rational) and
            isinstance(denominator, numbers.Rational)):
            numerator, denominator = (
                numerator.numerator * denominator.denominator,
                denominator.numerator * numerator.denominator
                )
        else:
            raise TypeError("both arguments should be "
                            "Rational instances")
    
        if denominator == 0:
            raise ZeroDivisionError('Fraction(%s, 0)' % numerator)
        if _normalize:
            if type(numerator) is int is type(denominator):
                # *very* normal case
                g = math.gcd(numerator, denominator)
                if denominator < 0:
                    g = -g
            else:
>               g = _gcd(numerator, denominator)
E               NameError: name '_gcd' is not defined
__class__  = <class 'fractions.Fraction'>
_normalize = True
cls        = <class 'fractions.Fraction'>
denominator = 809
numerator  = 483
self       = <[AttributeError("_numerator") raised in repr()] Fraction object at 0x7ff9bc6753a0>
/usr/lib64/python3.9/fractions.py:164: NameError

This removed _gcd, but it is still called in:

cpython/Lib/fractions.py

Line 164 in 58a4054

g = _gcd(numerator, denominator)

[hroncok]

Reproducer:

class myint(int):
    def __mul__(self, other):
        return type(self)(int(self)*int(other))
    @property
    def numerator(self):
        return type(self)(super().numerator)
    @property
    def denominator(self):
        return type(self)(super().denominator)

Before:

>>> fractions.Fraction(myint(1), myint(2))
Fraction(1, 2)

After:

>>> fractions.Fraction(myint(1), myint(2))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.9/fractions.py", line 164, in __new__
    g = _gcd(numerator, denominator)
NameError: name '_gcd' is not defined

[mdickinson]

The relevant piece of Python code from fractions.py looks like this:

    if type(numerator) is int is type(denominator):
        # *very* normal case
        g = math.gcd(numerator, denominator)
        if denominator < 0:
            g = -g
    else:
        g = _gcd(numerator, denominator)

I wonder whether we should simply drop the type check: math.gcd should accept anything that supports __index__ (and that includes NumPy integer types and anything that implements numbers.Integral).

OTOH, math.gcd will return a plain int, and if we have some subclass myint of int, we might want all the arithmetic to stay in the domain of myints (though I'm not really sure _why_ we'd want that). In that case we'll need to restore the pure Python _gcd implementation.

[hroncok]

Naively implementing this code, I'd use isinstance(numerator, int) over type(numerator) is int. Is that strict type check really needed?

I could not find anywhere whether numerator and denominator of numbers.Rational need to be numbers.Integral. I would expect so, however this is not written in the documentation of numbers.Rational. If that's the case, I think checking it for being numbers.Integral and failing hard if not is a reasonable thing to do.

[mdickinson]

Apologies, I think I moved the discussion off-track. The first order of business should be to fix the regression. We can discuss behaviour changes after that.

I've opened #62509 as a quick fix for the regression.

[vstinner]

I tried but failed to write a test to mimick numpy.int64 type. I tried to build a type which implements numbers.Rational but don't inherit from int, but there are way too many methods that I have to implement :-(

Morever, installing numpy on a Python 3.9 virtual environment is quite tricky. Lhe latest Cython release (0.29.14) isn't compatible with Python 3.9.

Miro gave me a command to install Cython on Python 3.9:

python -m pip install https://github.com/cython/cython/archive/master.tar.gz --install-option="--no-cython-compile"

But then "pip install numpy" tries to reinstall Cython which fails :-/

[vstinner]

I managed to test my PR with numpy:

$ env/bin/python 
>>> import fractions
>>> import numpy
>>> f=fractions.Fraction(numpy.int64(1*3), numpy.int64(2*3))
>>> f
Fraction(1, 2)
>>> type(f.numerator)
<class 'numpy.int64'>
>>> type(f.denominator)
<class 'numpy.int64'>

So it works as expected: numerator and denominator have the expected type, and there is no exception ;-)

I used the following commands to get numpy in a Python 3.9 virtual environment:

./python -m venv env
env/bin/python -m pip install https://github.com/cython/cython/archive/master.tar.gz --install-option="--no-cython-compile"
curl -O https://files.pythonhosted.org/packages/40/de/0ea5092b8bfd2e3aa6fdbb2e499a9f9adf810992884d414defc1573dca3f/numpy-1.18.1.zip
unzip -d . numpy-1.18.1.zip
cd numpy-1.18.1/
../env/bin/python setup.py install

[vstinner]

FYI the full numpy test suite pass with PR 18375:

(env) vstinner@apu$ python runtests.py -v
(...)
======================= 9879 passed, 443 skipped, 180 deselected, 17 xfailed, 3 xpassed in 305.17s (0:05:05) =======================

[vstinner]

New changeset dc7a50d by Victor Stinner in branch 'master':
bpo-39350: Fix fractions for int subclasses (GH-18375)
dc7a50d

[vstinner]

Thanks Miro for the bug report and thanks Mark for the great review and discussion! It should now be fixed. I tested manually that my change fix numpy test suite.