__floordiv__ in module fraction fails with TypeError instead of returning NotImplemented

[ShashkovS]

BPO	25412
Nosy	@ShashkovS
PRs	bpo-35588: Speed up mod, divmod and floordiv operations for Fraction type #11322
Files	fractions_truediv_fix.patch: fractions module fix in floordiv and mod

^{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 2019-01-11.10:31:46.264>
created_at = <Date 2015-10-15.14:09:54.015>
labels = ['type-bug', 'library']
title = '__floordiv__ in module fraction fails with TypeError instead of returning NotImplemented'
updated_at = <Date 2019-01-11.10:31:46.263>
user = 'https://github.com/ShashkovS'

bugs.python.org fields:

activity = <Date 2019-01-11.10:31:46.263>
actor = 'ShashkovS'
assignee = 'none'
closed = True
closed_date = <Date 2019-01-11.10:31:46.264>
closer = 'ShashkovS'
components = ['Library (Lib)']
creation = <Date 2015-10-15.14:09:54.015>
creator = 'ShashkovS'
dependencies = []
files = ['40797']
hgrepos = ['321']
issue_num = 25412
keywords = ['patch']
message_count = 8.0
messages = ['253046', '253047', '253052', '253077', '253081', '253083', '253096', '333448']
nosy_count = 1.0
nosy_names = ['ShashkovS']
pr_nums = ['11322']
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25412'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4', 'Python 3.5', 'Python 3.6']

[ShashkovS]

__floordiv__ in module fraction fails with TypeError instead of returning NotImplemented when modulo is a class without __rtruediv__ or __mul__.

Code sample:

class Foo(object):
    def __rdivmod__(self, other):
        return 'rdivmod works'

from fractions import Fraction
a = Fraction(1,1)
b = Foo()
print(divmod(1, b))
print(divmod(a, b))

__divmod__ in Fraction is inherited from class Real (numbers.py):
def __divmod__(self, other):
return (self // other, self % other)

So __floordiv__ and __mod__ are called.

    def __floordiv__(a, b):
        """a // b"""
        return math.floor(a / b)

    def __mod__(a, b):
        """a % b"""
        div = a // b
        return a - b * div

__floordiv__ if fractions.py makes a true division, and __mod__ makes multiplication.

The following code will fix the problem:

    def __divmod__(self, other):
        if isinstance(a, numbers.Complex):
            return (self // other, self % other)
        else:
            return NotImplemented

[ShashkovS]

def __floordiv__(a, b):
        """a // b"""
        if isinstance(b, numbers.Complex):
            return math.floor(a / b)
        else:
            return NotImplemented

And the same for __mod__.

[vstinner]

Can you please write a patch? See https://docs.python.org/devguide/

[oscarbenjamin]

You should test the change with number types that don't use the number tower e.g. Decimal, sympy, gmpy2, mpf, numpy arrays etc. Few non stdlib types use the number ABCs so testing against numbers.Complex may cause a change in behaviour.

[ShashkovS]

OK,

then we should not change numbers.py.

And in fractions.py:

def __floordiv__(a, b):
        """a // b"""
        if isinstance(b, numbers.Complex) or hasattr(b, '__rtruediv__'):
            fr = a / b
            if fr != NotImplemented:
                return math.floor(a / b)
            else:
                return NotImplemented
        else:
            return NotImplemented

[ShashkovS]

Bad idea, just

def __floordiv__(a, b):
        """a // b"""
        if isinstance(b, numbers.Complex):
            return math.floor(a / b)
        else:
            return NotImplemented

If b is inherited from number, real, complex, Fraction and etc, then a of type Fraction knows, how do make a division.
Otherwise may be b has __rfloordiv__, that khows how to be divided by Fraction.

[ShashkovS]

...
def forward(a, b):
if isinstance(b, (int, Fraction)):
return monomorphic_operator(a, b)
elif isinstance(b, float):
return fallback_operator(float(a), b)
elif isinstance(b, complex):
return fallback_operator(complex(a), b)
else:
return NotImplemented
forward.__name__ = '__' + fallback_operator.__name__ + '__'
forward.__doc__ = monomorphic_operator.__doc__

        def reverse(b, a):
            if isinstance(a, numbers.Rational):
                # Includes ints.
                return monomorphic_operator(a, b)
            elif isinstance(a, numbers.Real):
                return fallback_operator(float(a), float(b))
            elif isinstance(a, numbers.Complex):
                return fallback_operator(complex(a), complex(b))
            else:
                return NotImplemented
...
so division is possible only with int, Fraction, float, complex, numbers.Rational, numbers.Real, numbers.Complex.
For all of them "isinstance(b, numbers.Complex)" is true

[ShashkovS]

This patch actually fixes the problem:

https://bugs.python.org/issue35588
3a374e0

from fractions import Fraction
import operator

class Goo:
    __radd__, __rdivmod__, __rfloordiv__, __rmod__, __rmul__, __rpow__, __rsub__, __rtruediv__ = [lambda a, b: 'ok'] * 8

for func in operator.add, operator.sub, operator.mul, operator.truediv, operator.pow, operator.mod, operator.floordiv, divmod:
print(func.__name__, func(Fraction(1), Goo()))