I've ported the GMPY module to Python 3 and found a problem comparing
Fraction to gmpy.mpq. mpq is the rational type in gmpy and knows how to
convert a Fraction into an mpq. All operations appear to work properly
except "Fraction == mpq".

"mpq == Fraction" does work correctly. gmpy's rich comparison routine
recognizes the other argument as Fraction and converts to an mpq value
properly. However, when "Fraction == mpq" is done, the Fraction argument
is converted to a float before gmpy's rich comparison is called.

The __eq__ routine in fractions.py is:

    def __eq__(a, b):
        """a == b"""
        if isinstance(b, numbers.Rational):
            return (a._numerator == b.numerator and
                    a._denominator == b.denominator)
        if isinstance(b, numbers.Complex) and b.imag == 0:
            b = b.real
        if isinstance(b, float):
            return a == a.from_float(b)
        else:
            # XXX: If b.__eq__ is implemented like this method, it may
            # give the wrong answer after float(a) changes a's
            # value. Better ways of doing this are welcome.
            return float(a) == b

Shouldn't __eq__ return NotImplemented if it doesn't know how to handle
the other argument? I changed "return float(a) == b" to "return
NotImplemented" and GMPY and Python's test suite passed all tests.

I used the same logic for comparisons between gmpy.mpf and Decimal and
they all work correctly. Decimal does return NotImplemented when it
can't convert the other argument.

(GMPY 1.10 alpha2 fails due to this issue.)

I agree this should be fixed.  The conversion to float in the else 
clause seems wrong to me: it can lose precision, making two unequal 
values compare equal.  I also agree that we should be getting 
NotImplemented here.

Do you have a patch available?

As an aside, I'm interested that you're implementing comparisons between 
mpf and Decimal;  how does that work?  Do you also implement binary 
arithmetic operations between mpf and Decimal?  What's the result type?

On Tue, Jul 7, 2009 at 1:26 AM, Mark Dickinson<report@bugs.python.org> wrote:
>
> Mark Dickinson <dickinsm@gmail.com> added the comment:
>
> I agree this should be fixed.  The conversion to float in the else
> clause seems wrong to me: it can lose precision, making two unequal
> values compare equal.  I also agree that we should be getting
> NotImplemented here.
>
> Do you have a patch available?

I'll upload a patch this evening.

>
> As an aside, I'm interested that you're implementing comparisons between
> mpf and Decimal;  how does that work?  Do you also implement binary
> arithmetic operations between mpf and Decimal?  What's the result type?

When I do binary operations (including comparison), I check the type
of both operands. If both operands are Integer (mpz, int, long), I
convert both operands to an mpz and then perform the operation.
Otherwise, if both operands are Rational (mpz, int, long, mpq,
Fraction), I convert both operands to an mpq. Finally, if both
operands appear to be a Number (mpz, int, long, mpq, Fraction, mpf,
Decimal), I convert both operands to an mpf. In the latest release of
GMPY, I always return a GMPY type (unless you are converting to float
or int/long).

To do the conversions, I just do mpq(str(Fraction)) or
mpf(str(Decimal)). I realize that the conversions aren't perfect but
are probably what a user expects. The fact that GMP uses radix 2^32 or
2^64 floating point with (randomly) either [prec+1] or [prec+2] digits
makes predictable floating point challenging. ;-) (prec =
floor(precision_in_bits/limb_size).) In GMPY 1.04, I forced all mpf
results to have [prec+1] digits but it's still messy. Time to start
working on MPFR support.
>
> ----------
> nosy: +jyasskin, marketdickinson
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue6431>
> _______________________________________
>

Change Fraction __eq__ method to give the other operand a chance to
perform the comparison if Fraction doesn't understand the other operand.

Thanks.  Could you provide some tests?  (If you don't have time then 
that's fine: I'll probably get to this eventually, but it might take a 
while...)

A good solution should ensure that all 6 comparison operators behave in 
the same way for the same input types:  that is, if x == y returns 
NotImplemented for some particular Python objects x and y, then x < y, x 
<= y, etc. should also return NotImplemented, and vice versa.

I've attached a patch that creates DummyRational and then tests
comparisons between Fraction and DummyRational. The __eq__ method also
verifies that the type of the other argument is fractions.Fraction.

Thanks again, casevh!  The patch looks good.  I've added to it a bit,
though---see issue6431.patch.  In detail:

- don't use subtraction with unknown types for <, <=, >, >=;  this is
  dangerous, since the unknown type may well do a lossy conversion, and
  comparisons should really be exact where possible;  as with __eq__,
  it seems better to return NotImplemented and give the other type a
  chance.

- handle infs and nans correctly in comparisons with floats

- a few more tests.

casevh, please could you have a look at the attached patch and let me
know whether it still works with your gmpy port?

Jeffrey, any comments on these changes?

The fallback behavior in Fraction was meant to demonstrate the suggested 
fallback behavior for user-defined types. In particular, the idea was 
that all Reals would (by default) be comparable to each other, even if 
they didn't know about each other. Unfortunately, the comparison protocol 
provides no way to know if a particular call is the first call or the 
fallback, so __eq__(Fraction, b) has to return the same thing as 
__eq__(b, Fraction). If b doesn't know about Fraction, and Fraction wants 
to make a best attempt at comparing to it, then __eq__(Fraction, b) can't 
return NotImplemented.

[Jeffrey]
> In particular, the idea was that all Reals would (by default) be
> comparable to each other, even if they didn't know about each other.

Understood, but I don't think this is an attainable goal.  I don't see 
any reasonable way to make it happen without doing significant guessing, 
or expanding the numbers ABC in some way.

At the moment, __eq__(a, b) falls back to 'float(a) == b' when b is not 
a float or an instance of Rational.  This seems problematic to me for a 
couple of reasons:

1. The conversion to float loses information.  As a result, we lose (a) 
transitivity of equality, (b) well-behaved hashing (x == y no longer 
implies hash(x) == hash(y)), and (c) consistency between == and the 
other comparison operators.

2. This fallback shuts out the other class even in cases where the other 
class *does* know how to handle the comparison.  So there's no way for 
another class to 'play nice' with the Fraction type and implement exact 
comparisons even if it wants to.

Here's an example of 1, on Python 2.6.  (bigfloat is a home-built 
wrapper for the MPFR library.)

newton:~ dickinsm$ python2.6
Python 2.6.2 (r262:71600, Jun 17 2009, 09:08:27) 
[GCC 4.0.1 (Apple Inc. build 5490)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from bigfloat import BigFloat
>>> from fractions import Fraction
>>> x = Fraction(2**60-1)
>>> y = Fraction(2**60+1)
>>> z = BigFloat(2**60)
>>> x == z
True
>>> y == z
True
>>> x == y
False
>>> hash(x) == hash(z)
False

I just don't see any reasonable way to make comparisons 'automatically' 
work:  one of the classes has to know how to handle both types, or else 
there's just going to be a lot of guesswork involved.  So it seems 
better to simply return NotImplemented in these cases.

If you think it's better, I'm happy to make the other tradeoff.

On Thu, Jul 16, 2009 at 11:34 AM, Mark Dickinson<report@bugs.python.org> wrote:
>
> Mark Dickinson <dickinsm@gmail.com> added the comment:
>
> Thanks again, casevh!  The patch looks good.  I've added to it a bit,
> though---see issue6431.patch.  In detail:
>
> - don't use subtraction with unknown types for <, <=, >, >=;  this is
>  dangerous, since the unknown type may well do a lossy conversion, and
>  comparisons should really be exact where possible;  as with __eq__,
>  it seems better to return NotImplemented and give the other type a
>  chance.
>
> - handle infs and nans correctly in comparisons with floats
>
> - a few more tests.
>
> casevh, please could you have a look at the attached patch and let me
> know whether it still works with your gmpy port?

I've tested gmpy with attached patch and all tests pass successfully.
Thanks!

>
> Jeffrey, any comments on these changes?
>
> ----------
> stage: test needed -> patch review
> Added file: http://bugs.python.org/file14508/issue6431.patch
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue6431>
> _______________________________________
>

Applied in r74078 (py3k), r74079 (release31-maint).  I'll backport to 2.x.

Backported to trunk in r74080.  I don't think it's worth fixing this in 
2.6:  it seems unlikely that the changed comparison behaviour would cause 
breakage, but I don't want to take the chance.

Thanks Case for the report and patches!