Fraction modulo infinity should behave consistently with other numbers

[elias6]

BPO	32968
Nosy	@rhettinger, @mdickinson, @elias6, @serhiy-storchaka
PRs	bpo-32968: Modulo and floor division involving Fraction and float should return float #5956

^{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 = 'https://github.com/mdickinson'
closed_at = <Date 2018-08-27.07:13:21.021>
created_at = <Date 2018-02-28.03:10:51.433>
labels = ['interpreter-core', '3.8']
title = 'Fraction modulo infinity should behave consistently with other numbers'
updated_at = <Date 2018-08-27.07:13:21.019>
user = 'https://github.com/elias6'

bugs.python.org fields:

activity = <Date 2018-08-27.07:13:21.019>
actor = 'mark.dickinson'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2018-08-27.07:13:21.021>
closer = 'mark.dickinson'
components = ['Interpreter Core']
creation = <Date 2018-02-28.03:10:51.433>
creator = 'elias'
dependencies = []
files = []
hgrepos = []
issue_num = 32968
keywords = ['patch']
message_count = 23.0
messages = ['313045', '313059', '313081', '313088', '313089', '313092', '313103', '313113', '313114', '313135', '313154', '313239', '313271', '313833', '313854', '313864', '313932', '314108', '314353', '324080', '324142', '324151', '324152']
nosy_count = 5.0
nosy_names = ['rhettinger', 'mark.dickinson', 'jyasskin', 'elias', 'serhiy.storchaka']
pr_nums = ['5956']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue32968'
versions = ['Python 3.8']

[elias6]

Usually, a positive finite number modulo infinity is itself. But modding a positive fraction by infinity produces nan:

>>> from fractions import Fraction
>>> from math import inf
>>> 3 % inf
3.0
>>> 3.5 % inf
3.5
>>> Fraction('1/3') % inf
nan

Likewise, a positive number modulo negative infinity is usually negative infinity, a negative number modulo infinity is usually infinity, and a negative number modulo negative infinity is usually itself, unless the number doing the modding is a fraction, in which case it produces nan.

I think fractions should behave like other numbers in cases like these. I don't think this comes up very often in practical situations, but it is inconsistent behavior that may surprise people.

I looked at the fractions module. It seems like this can be fixed by putting the following lines at the top of the __mod__ method of the Fraction class:

    if b == math.inf:
        if a >= 0:
            return a
        else:
            return math.inf
    elif b == -math.inf:
        if a >= 0:
            return -math.inf
        else:
            return a

If that is too verbose, it can also be fixed with these lines, although this is less understandable IMO:

    if math.isinf(b):
        return a if (a >= 0) == (b > 0) else math.copysign(math.inf, b)

I noticed this in Python 3.6.4 on OS X 10.12.6. If anyone wants, I can come up with a patch with some tests.

[mdickinson]

I'm not quite sure why Fraction_instance % float_instance wouldn't simply convert the Fraction to a float and then use the usual floating-point mod. There's the issue that you want to maintain consistency with the floordiv operation, but if Fraction_instance // float_instance also converts its first argument to float, then that consistency should follow automatically.

That's the pattern that's followed for most of the other binary arithmetic operators. It looks like the inconsistency is caused by using _operator_fallbacks for most of the operations, but doing something more ad-hoc for modulo.

I agree that the current behaviour is surprising.

[elias6]

Mark, you have some good ideas. A fraction modulo a float is a float, and an integer modulo infinity produces itself as a float, so it seems reasonable that a fraction modulo infinity should be itself converted to a float.

I tried assigning __mod__ and __rmod__ using _operator_fallbacks, like most of the binary operators, and calculations involving infinity behaved as I expected, but one test failed (1.0 % Fraction(1, 10) was no longer 0 because of rounding error).

But then I tried assigning only __mod__ using _operator_fallbacks, and leaving __rmod__ alone, and all the tests passed (including some ones I added to make sure that modulo calculations involving infinity behave like both of us think they should).

As for the floordiv operator, I'm not sure what to think. It would be a bit strange if mod with a float returns a float and floordiv with a float returns an int. It wouldn't be a big deal IMO but it may be easy to change. I tried a simple change to make floordiv with a float to return a float, and changed one test to make sure that it works, and all of the tests passed.

I can make a pull request if anyone wants.

[mdickinson]

It would be a bit strange if mod with a float returns a float and floordiv with a float returns an int.

Agreed: if there are floats involved (Fraction // float or float // Fraction), I think the rule should be that we simply fall back to float behaviour, and do whatever float // float does - i.e., return a float. (I'd actually prefer that a mixed Fraction-float operation be a TypeError, but that ship sailed long ago.)

OTOH, if there's a test that's explicitly checking that my_fraction // my_float returns something of type int, then the behaviour is clearly intentional. That means that we should be cautious about changing it, and also means that we probably can't change it in 3.6 or 3.7; it'll have to wait for 3.8. (But I think that's fine; this seems like a minor consistency cleanup that I would expect to affect very few people.)

It looks as though this was a part of PEP-3141 that was never fully implemented: the "Real" type there has a __floordiv__ method that looks like this:

    @abstractmethod
    def __floordiv__(self, other):
        """The floor() of self/other. Integral."""
        raise NotImplementedError

But float // float does *not* currently return an Integral:

    >>> import numbers
    >>> x = 2.3
    >>> isinstance(x, numbers.Real)
    True
    >>> isinstance(x // x, numbers.Integral)
    False

And that definitely shouldn't change: I'd argue against such a change in any case, but backwards compatibility considerations alone mean that we shouldn't change this now to return an integer. Given that, I think it's acceptable to have a mixed fraction-float floor division return a float.

A pull request would be great. Yes, please!

[mdickinson]

I'd argue against such a change in any case

And apparently I already did: see bpo-22444 for previous discussion on the topic.

[serhiy-storchaka]

Not all Fractions can be converted to float.

>>> Fraction(2**2000, 3) // 1.0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/fractions.py", line 432, in __floordiv__
    return math.floor(a / b)
  File "/home/serhiy/py/cpython/Lib/fractions.py", line 378, in forward
    return fallback_operator(float(a), b)
  File "/home/serhiy/py/cpython/Lib/numbers.py", line 291, in __float__
    return self.numerator / self.denominator
OverflowError: integer division result too large for a float

What is surprising that the modulo operation can fail even if the end result could be converted to float.

>>> Fraction(2**2000, 3) % 1
Fraction(1, 3)
>>> Fraction(2**2000, 3) % 1.0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/fractions.py", line 440, in __mod__
    div = a // b
  File "/home/serhiy/py/cpython/Lib/fractions.py", line 432, in __floordiv__
    return math.floor(a / b)
  File "/home/serhiy/py/cpython/Lib/fractions.py", line 378, in forward
    return fallback_operator(float(a), b)
  File "/home/serhiy/py/cpython/Lib/numbers.py", line 291, in __float__
    return self.numerator / self.denominator
OverflowError: integer division result too large for a float

[mdickinson]

Serhiy: I think both those results are reasonable, and not too surprising. It should be easy to understand that the mixed-type operation converts one of the arguments to float, and if that conversion overflows we get an exception. A similar thing happens when doing something like math.sqrt(2**1024), even though the result is representable.

[elias6]

Mark, you said "if there's a test that's explicitly checking that my_fraction // my_float returns something of type int, then the behaviour is clearly intentional". I see exactly that. See

cpython/Lib/test/test_fractions.py

Line 404 in bf63e8d

self.assertTypedEquals(0, F(1, 10) // 1.0)

In fact, when I said I made floordiv return a float, and "changed one test to make sure that it works", it was that test. So maybe that means we should be more cautious about changing the behavior of floordiv, if we decide to change it.

[elias6]

I added the pull request. It includes only my changes to modulo, and not the ones to floordiv.

[mdickinson]

[Elias Zamaria]

I see exactly that.

Understood; that's how I interpreted your "changed one test to make sure that it works". It seems we understand each other. :-)

So yes, one always needs to be very cautious about changing deliberate, tested behaviour. But in this case I'm satisfied that I understand what the intent was when that test was written, namely that Real.__floordiv__ returns an Integral. And since that part of PEP-3141 didn't actually get implemented for float // float (and the agreement on bpo-22444 was that the PEP should be changed rather than the behaviour), that intent is now a little bogus, and I think it's reasonable to change the behaviour in this case. The change would make the overall behaviour of mixed-type operations simple to state and understand, and fix a consistency bug. It's also sufficiently much of a corner case that the change seems unlikely to break existing code.

In short, I'm +1 on making your suggested change to __floordiv__ as well as to __mod__.

I'm adding Jeffrey Yasskin (the original PEP-3141 author) to the nosy list, in case he has any comment. (I don't believe Jeffrey is still watching Python development, but would be happy to be proved wrong.)

[elias6]

Any suggestions as to what I should do? I can either update my pull request with my floordiv change, or create a new pull request, or wait a while to see if anyone else has any opinion on changing the behavior.

[mdickinson]

Elias: please do go ahead and update your PR.

[elias6]

Done.

[mdickinson]

Thanks for the PR (and apologies for being slow to look at it). I think we want to use the operator_fallbacks approach for both __rfloordiv__ and __floordiv__ (and similarly for __rmod__ and __mod__), else we'll get inconsistent results:

>>> Fraction(3) // 5.0  # get float, as expected
0.0
>>> 3.0 // Fraction(5)  # expect a float, get an integer
0

Please could you make that change and add a couple more tests that cover this case?

[elias6]

Mark, what you described (operator_fallbacks for both __rfloordiv__ and __floordiv__, and for both __rmod__ and __mod__) was my initial approach. But that broke one test (which floor-divides 1.0 by 1/10 and expects the result to be an integer). I thought about fixing it, to make the behavior more consistent, but I thought that was a bit risky.

Just now, I tried the change again, as you suggested, but I fixed the test to expect a result of 10.0 (a float) instead of 10 (an integer). I got a strange result from that test, saying the result was 9.0.

It seems like this is caused by rounding error, since operator_fallbacks converts both numbers to floats if one of them is a float, and 1/10 can't be represented exactly as a float, so it gets rounded to slightly more than 1/10:

>>> float(Fraction(1, 10)).as_integer_ratio()
(3602879701896397, 36028797018963968)
>>> Decimal.from_float(float(Fraction(1, 10)))
Decimal('0.1000000000000000055511151231257827021181583404541015625')

So yes, I can make that change, but I'm not sure if it would be a good idea. Do you have any thoughts?

[mdickinson]

Yes, that sort of thing is going to happen as soon as floating-point enters the mix. There will be surprises from Fraction % float as well as float % Fraction:

>>> from fractions import Fraction
>>> Fraction(10**23) // 1e22
9.0

And that's again surprising, because 1e22 is exactly equal to 10**22:

>>> 1e22 == 10**22
True

This isn't special to Fractions: the same is true for mixed-type int-float arithmetic:

>>> 10**23 // 1e22
9.0

As you say, this is the result of rounding error.

There's not much we can do about that except for make sure that people are aware that precision can be lost when a Fraction is converted to a float. (One could conceivably outlawed mixed-type Fraction-float operations altogether, but rightly or wrongly that's not the decision that was made when the Fraction type was introduced, and changing that now would amount to gratuitous breakage.)

So yes, modifying both __floordiv__ and __rfloordiv__ together (and similarly for __mod__) is the right thing to do: we definitely don't want Fraction % float and float % Fraction to return different types.

On the testing front, testing the result value of something like 1.0 // Fraction(1, 10) is a little bit dodgy, because the result is only predictable under assumptions that we're using IEEE 754 arithmetic, and that's (at the moment) not an assumption that the Python core makes. I'd suggest using a safer case like 1.0 // Fraction(3, 10).

I thought about fixing it, to make the behavior more consistent, but I thought that was a bit risky.

I think you have excellent instincts here. :-) But in this case, I do think it's better to be consistent, and to have a easy-to-learn and simple-to-remember rule (mixed-type Fraction-float arithmetic operations convert the Fraction to a float, regardless of the operation or the ordering of the operands). The floating-point surprises are a fact of life anyway, regardless of what Fraction does.

[elias6]

Mark, I tried Fraction(10**23) // 1e22, and I got 10.

Your 10**23 // 1e22 example was strange, but then int(1e23) and got 99999999999999991611392. That is kind of unexpected but I think it is rare for anyone to do something like that with numbers that big.

I think the fact that floating-point rounding error sometimes causes strange results is not a reason to do really unexpected things like making 1.0 // 1/10 equal 9.0, if that can be reasonably avoided.

I updated my pull request with my change, which you suggested, to make __rfloordiv__ and __rmod__ return a float, but with a small change to _operator_fallbacks to avoid the rounding error, so 1.0 // 1/10 is 10.0. You can see it at 1020bb2#diff-14d03bfb59581367725b00781e6f802fL391. What do you think?

[mdickinson]

Mark, I tried Fraction(10**23) // 1e22, and I got 10.

Sorry: that result was with your PR (as it was at the time I wrote that comment). On master, you do indeed get 10.

I think the fact that floating-point rounding error sometimes causes
strange results is not a reason to do really unexpected things like
making 1.0 // 1/10 equal 9.0, if that can be reasonably avoided.

I understand, but I think in this case, the cure is worse than the disease. I don't think converting the float to a Fraction in mixed-type operations is going to work, for a number of reasons:

• it's changing the behaviour for _all_ the arithmetic operations, not just // and %; that widens the scope for accidental breakage. For example, with the latest commit 038664e on your PR:

>>> Fraction(1) + math.inf
inf
>>> math.inf + Fraction(1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/mdickinson/Python/cpython/Lib/fractions.py", line 391, in reverse
    return float(fallback_operator(Fraction(a), b))
  File "/Users/mdickinson/Python/cpython/Lib/fractions.py", line 130, in __new__
    self._numerator, self._denominator = numerator.as_integer_ratio()
OverflowError: cannot convert Infinity to integer ratio

But on master, we have:

>>> import math
>>> from fractions import Fraction
>>> math.inf + Fraction(1)
inf
>>> Fraction(1) + math.inf
inf

• it's counter to the spirit of the numeric tower, where for most arithmetic operations, values are propagated _up_ the tower (from Integral -> Rational -> Real -> Complex) to the first common type before the operation is performed. That keeps things simple; having some cases where values are converted down the tower is going to cause confusion. (Comparisons are necessarily a special case: the need to preserve transitivity of equality and trichotomy means that using the same rules as for arithmetic operations won't fly)

• it introduces a non-obvious difference between mixed-mode Fraction-float and int-float operations. For an int x and a float y, I'd expect the result of x <some_op> y to be identical to the result of Fraction(x) <some_op> y.

So I'm sorry, but I do think having 1.0 // Fraction(1, 10) give 9.0 rather than 10.0 is the least worst option here. It's a surprise, but it's not a _new_ surprise: it's a surprise that's already there in the world of floating-point arithmetic.

>>> 1.0 // 0.1
9.0

You're evaluating a discontinuous function _at_ a discontinuity, using a type that's known for its inexactness. In that situation, getting the value for _either_ side of that discontinuity is reasonable.

[elias6]

Mark, you have some good points. I didn't fully think about the implications of my change. I undid the change to _operator_fallbacks.

I updated the tests to expect 1.0 // 1/10 to equal 9.0 and 1.0 % 1/10 to equal 0.09999999999999995. That latter number seems a bit awkward though. Can I expect the result to always come out like that, or could it depend on the hardware the test is run on? If we can't depend on that result, do you have any suggestions?

[mdickinson]

Sorry that I've taken so long to get back to this. I've just updated the PR, and I think it's ready to go. Looks like it does need you to update your GitHub username here in the "Your Details" section of the bugtracker, though; sorry about that.

If we can't depend on that result, do you have any suggestions?

Yes: check that 1.0 // 1/10 equals 1.0 // 0.1, and similarly for %. I've made that change in the PR.

[elias6]

I updated my GitHub username. For the record, it used to be mikez302, and now it is elias6.

[mdickinson]

New changeset 393f1ff by Mark Dickinson (Elias Zamaria) in branch 'master':
bpo-32968: Make modulo and floor division involving Fraction and float consistent with other operations (bpo-5956)
393f1ff

[mdickinson]

Fixed for Python 3.8. Thank you!