Issue39350
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2020-01-16 08:13 by vstinner, last changed 2022-04-11 14:59 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 18021 | merged | vstinner, 2020-01-16 08:17 | |
| PR 18309 | closed | mark.dickinson, 2020-02-02 10:06 | |
| PR 18375 | merged | vstinner, 2020-02-06 12:42 | |
| Messages (12) | |||
|---|---|---|---|
| msg360095 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-01-16 08:13 | |
bpo-22486 added math.gcd() and deprecated fractions.gcd() in Python 3.5: commit 48e47aaa28d6dfdae128142ffcbc4b0642422e90. The function was deprecated during 4 cycles (3.5, 3.6, 3.7, 3.8): I propose attached PR to remove it. |
|||
| msg360111 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-01-16 10:02 | |
New changeset 4691a2f2a2b8174a6c958ce6976ed5f3354c9504 by Victor Stinner in branch 'master': bpo-39350: Remove deprecated fractions.gcd() (GH-18021) https://github.com/python/cpython/commit/4691a2f2a2b8174a6c958ce6976ed5f3354c9504 |
|||
| msg361103 - (view) | Author: Miro Hrončok (hroncok) * | Date: 2020-01-31 12:52 | |
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:
https://github.com/python/cpython/blob/58a4054760bffbb20aff90290dd0f3554f7bea42/Lib/fractions.py#L164
|
|||
| msg361104 - (view) | Author: Miro Hrončok (hroncok) * | Date: 2020-01-31 13:09 | |
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
|
|||
| msg361111 - (view) | Author: Mark Dickinson (mark.dickinson) * ![]() |
Date: 2020-01-31 14:44 | |
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.
|
|||
| msg361126 - (view) | Author: Miro Hrončok (hroncok) * | Date: 2020-01-31 19:30 | |
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. |
|||
| msg361225 - (view) | Author: Mark Dickinson (mark.dickinson) * ![]() |
Date: 2020-02-02 10:08 | |
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 GH-18309 as a quick fix for the regression. |
|||
| msg361481 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-02-06 13:42 | |
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 :-/ |
|||
| msg361483 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-02-06 14:19 | |
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 |
|||
| msg361485 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-02-06 14:34 | |
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) ======================= |
|||
| msg361612 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-02-07 22:42 | |
New changeset dc7a50d73a3d16918529669ff7b8783c08cff090 by Victor Stinner in branch 'master': bpo-39350: Fix fractions for int subclasses (GH-18375) https://github.com/python/cpython/commit/dc7a50d73a3d16918529669ff7b8783c08cff090 |
|||
| msg361613 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-02-07 22:44 | |
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. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:25 | admin | set | github: 83531 |
| 2020-02-07 22:44:10 | vstinner | set | status: open -> closed resolution: fixed messages: + msg361613 stage: patch review -> resolved |
| 2020-02-07 22:42:55 | vstinner | set | messages: + msg361612 |
| 2020-02-06 14:34:29 | vstinner | set | messages: + msg361485 |
| 2020-02-06 14:19:57 | vstinner | set | messages: + msg361483 |
| 2020-02-06 13:42:26 | vstinner | set | resolution: fixed -> (no value) messages: + msg361481 |
| 2020-02-06 12:42:10 | vstinner | set | pull_requests: + pull_request17751 |
| 2020-02-02 10:08:55 | mark.dickinson | set | messages: + msg361225 |
| 2020-02-02 10:06:39 | mark.dickinson | set | stage: resolved -> patch review pull_requests: + pull_request17685 |
| 2020-01-31 19:30:00 | hroncok | set | messages: + msg361126 |
| 2020-01-31 14:44:47 | mark.dickinson | set | status: closed -> open messages: + msg361111 |
| 2020-01-31 14:19:55 | mark.dickinson | set | nosy:
+ mark.dickinson |
| 2020-01-31 13:09:43 | hroncok | set | messages: + msg361104 |
| 2020-01-31 12:52:22 | hroncok | set | nosy:
+ hroncok messages: + msg361103 |
| 2020-01-16 10:03:19 | vstinner | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2020-01-16 10:02:58 | vstinner | set | messages: + msg360111 |
| 2020-01-16 08:17:23 | vstinner | set | keywords:
+ patch stage: patch review pull_requests: + pull_request17416 |
| 2020-01-16 08:13:44 | vstinner | create | |
