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 |