Remove deprecated fractions.gcd()

@mdickinson

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']

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.

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

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)

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

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.

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.

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.

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 :-/

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

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) =======================

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

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.

Navigation Menu

Remove deprecated fractions.gcd() #83531

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Remove deprecated fractions.gcd() #83531

Description

Activity

vstinner commented on Jan 16, 2020

vstinner commented on Jan 16, 2020

hroncok commented on Jan 31, 2020

hroncok commented on Jan 31, 2020

mdickinson commented on Jan 31, 2020

hroncok commented on Jan 31, 2020

mdickinson commented on Feb 2, 2020

vstinner commented on Feb 6, 2020

vstinner commented on Feb 6, 2020

vstinner commented on Feb 6, 2020

vstinner commented on Feb 7, 2020

vstinner commented on Feb 7, 2020

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions