float compared to decimal is silently incorrect.

[jdunck]

BPO	2531
Nosy	@rhettinger, @facundobatista, @mdickinson, @skrah
Files	decimal.patch: always raise exception if 'other' can't be converted float_decimal_comparisons.patch issue2531.patch

^{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 2010-04-03.11:45:02.858>
created_at = <Date 2008-04-01.22:11:36.754>
labels = ['type-bug', 'library']
title = 'float compared to decimal is silently incorrect.'
updated_at = <Date 2010-04-03.11:45:02.857>
user = 'https://bugs.python.org/jdunck'

bugs.python.org fields:

activity = <Date 2010-04-03.11:45:02.857>
actor = 'mark.dickinson'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2010-04-03.11:45:02.858>
closer = 'mark.dickinson'
components = ['Library (Lib)']
creation = <Date 2008-04-01.22:11:36.754>
creator = 'jdunck'
dependencies = []
files = ['9922', '13389', '16544']
hgrepos = []
issue_num = 2531
keywords = ['patch']
message_count = 35.0
messages = ['64827', '64855', '83691', '83694', '83816', '83818', '83935', '83939', '83940', '83941', '83942', '83944', '83948', '83950', '83951', '83952', '84433', '85099', '86053', '86058', '86059', '86061', '86123', '86438', '86439', '95263', '97891', '97935', '98215', '98217', '98218', '101053', '101231', '102147', '102242']
nosy_count = 9.0
nosy_names = ['rhettinger', 'facundobatista', 'mark.dickinson', 'jdunck', 'lorg', 'skrah', 'adamtj', 'bertchughes', 'bas']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue2531'
versions = ['Python 3.2']

[jdunck]

Within python 2.5.2:

>>> from decimal import Decimal
>>> x = 3.0
>>> y = Decimal('0.25')
>>> x > y
False (expected error, as in 2.4, or True)

[facundobatista]

Which version of 2.4 are you talking about?

This line in the tests...

  self.assertNotEqual(da, 32.7)

is there since almost 4 years ago (release 36325, and 2.4 is tagged in
release 37906).

Anyway, this behaviour is ok, as is the following:

>>> 2 < "1.9"
True

[lorg]

This behavior also exists in Python 2.6. However, in Python 3 an
exception is raised instead, as these kinds of comparisons are
considered incorrect.
Because of that, I'd like Python 3's behavior of raising exceptions on
float-decimal comparisons to be backported to Python 2.6.

As an aside, regardless of Python 3's behavior, there is a big
difference between 2<"1.9" and 1.6 < decimal("2.0"). It seems reasonable
to expect decimal comparisons to be made according to numerical value.
When this doesn't happen, it is better to fail loudly rather than silently.

[rhettinger]

Imri, I don't think the 3.0 model for cross-type comparisons can be
backported without breaking code, so it probably isn't going to happen.
The whole purpose of the 3.x series was to allow improvements that
weren't backwards compatible.

Mark, this raises a question for us. Now that we have
decimal.from_float(), we do have a means of making exact comparisons
between decimals and floats. While I think cross-type interaction is
generally a bad idea, the 2.x way of doing things already gives an
answer (though somewhat useless). What are your thoughts on making
Decimal('0.80') < float('0.75') do the right thing in the 2.x series so
that the answer that is given won't be flat-out wrong.

[mdickinson]

Making float <-> Decimal comparisons return the 'right' answer in 2.x does
look attractive at first sight, but the more I think about it the more it
seems a bad idea. Having the comparisons work in 2.x but not in 3.x seems
especially nasty, and allowing mixed-type comparisons but not mixed-type
arithmetic operations also seems somehow unclean. So -1 from me. (And
no, I don't want to add full float <-> Decimal interaction: the Decimal
module is quite complicated enough as it is.)

Are there many cases where float <-> Decimal comparisons are useful? The
only uses I can think of would also involve a need for mixed-type
arithmetic. In the few cases where it's really needed I don't think it's
a problem to explicitly convert one or the other type.

The current bogus comparison results also suck, but they're just one
aspect of a known Python 2.x gotcha.

Would it be possible to raise a warning for Decimal <-> float comparisons?
Does -3 already do this?

[jdunck]

I hear you on the 2.x to 3.x transition-- I'm not really asking for
mixed-mode arithmetic. I'd be perfectly happy if float > decimal raised
TypeError, as float + decimal does.

My complaint is that it is silently wrong.

[mdickinson]

My complaint is that it is silently wrong.

I appreciate that, but I don't see any good solution. Yes, it would be
nice if float <-> Decimal comparisons raised TypeError in 2.x. But Python
2.x has an '(almost) everything is comparable to everything else'
comparison model, so that for example a list of arbitrary Python objects
can almost always be sorted. So raising a TypeError for these comparisons
has the potential to break already existing code. Of course, it *might*
not cause any breakage at all, but it's difficult to see how one could
ever be sure of that.

I think the best we can do would be to add a warning for float <-> Decimal
comparisons. Facundo, Raymond: does this seem reasonable?

[rhettinger]

I think "the best we can do" is return valid comparison results between
floats and decimals in 2.x. It doesn't make anything worse and it does
make something better. Unlike other cross-type comparisons,
number-to-number is not an unreasonable thing to do. And, we not
obliged to carry that over to 3.x where cross-type comparisons have to
be specifically enabled. I believe this is the best solution.

[rhettinger]

The next to last sentence should have read "and, we are not
obliged to carry that over to 3.x where cross-type ordering comparisons
are not the norm unless a type has specifically enabled them."

The gist of the idea is that in 2.x, we do have cross-type ordering
comparisons so we should make them as useful and non-misleading as
possible. But, in 3.x there is no default cross-type ordering
comparisons so we're not obliged to return any answer at all.

[mdickinson]

What about Decimal <-> Fraction comparisons?

[rhettinger]

It's not a priority for me though it's not an unreasonable thing to do.
A basic 5th grade exercise is ordering fractions, sometimes with their
decimal equivalents: Fraction(1,3) < Decimal('0.4') < Fraction(1,2).
I don't care if that gets done.

If you do decide to do fractions too, the responsibility should be with
the fractions class (since decimals convert to fractions exactly but not
vice-versa).

[mdickinson]

Here's a patch.

I'm still not 100% convinced this is a good idea. Part of my objection is
that it seems likely that these comparisons are fairly useless, in that a
mixed-type comparison is probably going to be followed by a mixed-type
arithmetic operation at some point (unless people are misspelling "x < 0"
as "x < 0.0"). So all that's really gained is a noisy failure instead of
a silent one. Still, I suppose that's something.

[mdickinson]

On the other hand, if it's true that mixed-type comparisons are generally
followed by mixed-type arithmetic, then these comparisons just become a
roundabout way to raise TypeError, which is what everybody wanted in the
first place. :-)

The patch still needs docs.

[mdickinson]

Urk. That patch produces horrible results when comparing Fractions and
Decimals:

Python 2.7a0 (unknown, Mar 21 2009, 17:59:48) 
[GCC 4.0.1 (Apple Inc. build 5490)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from fractions import Fraction
>>> from decimal import Decimal
>>> Decimal('2.5') == Fraction(5, 2)
True
>>> Decimal('1.1') == Fraction(11, 10)
False

Either both results should be True (if comparisons between Fractions and
Decimals work numerically), or both should be False (refuse to
compare Fraction and Decimal).

It looks like what happens is that the Fraction comparison converts the
second argument to float before comparing. I'm tempted to call this a
bug in Fraction.__eq__.

[mdickinson]

One more consideration: if Decimal('2.5') == 2.5 is True, should we also
be fixing the Decimal hash method so that hash(Decimal('2.5')) ==
hash(2.5)?

[rhettinger]

I'll look at this more later today.

[mdickinson]

Removing easy keyword, since I don't think it applies here.

The problem here is hashing: either we break the rule that
if two objects compare equal then they hash equal, or we
fix hash so that e.g., hash(Decimal('2.5')) == hash(2.5).

For the latter, the least invasive way to do it would be
to fix only the Decimal __hash__ method. For that, we
really need a Decimal -> float conversion, so that we
can do something like (for a Decimal x):

if x == Decimal.from_float(x.to_float()):
    return hash(x.to_float())
[rest of hash method here]

The builtin float() (which converts a Decimal to a string
and then uses the standard C library's string -> float
conversion) probably isn't good enough for this, since
there are no requirements that it should be (even close to)
correctly rounded.

The bottom line: getting a correctly-rounded
Decimal -> float method, without knowing what
the float format is, is going to be hard.
If we assume IEEE 754 then it's much easier.

[rhettinger]

Mark, I looked at your patch again and think we should support relaxed
conversions for ordering comparisons but not equality/inequality. This
will improve on the current situation where we get flat-out misleading
results for <, <=, >, and >=. It keeps the status quo for
equality/inequality and thereby avoids the problems with __hash__.

The only thing that I don't like about it is that you can't do the usual
reasoning where "not a<b and not a>b" implies "a==b". Still, it is an
improvement over "a<b" giving a completely useless and misleading result.

[rhettinger]

Mark, any thoughts? I would like to apply this patch for ordering
comparisons other than __eq__ and __ne__.

[mdickinson]

On Fri, Apr 17, 2009 at 3:45 AM, Raymond Hettinger
<report@bugs.python.org> wrote:

Mark, any thoughts?  I would like to apply this patch for ordering
comparisons other than __eq__ and __ne__.

Hi Raymond,

Sorry for not responding to this sooner. I'll post something to
the issue tracker. In brief, this seems an improvement over
the earlier proposal, just because there are no problems with
hashes or container confusion. I'm still -0 on it, though---it
just seems like too much magic: confusion will happen
much more rarely, but it'll be that much more difficult to
explain when it does occur.

IOW I wouldn't do this if it were just up to me, but not going
to object if there's support from you and others.

Mark

[rhettinger]

I'm not seeing the downside. This gives better answers than it does now
but doesn't commit us to anything else.

[mdickinson]

The downside is the potential confusion arising from
using one method (comparison of actual numerical value)
for <, <=, >, >=, and a different method (decimals
and floats are never equal) for == and !=.

[mdickinson]

Unassigning myself.

Does anyone beside Raymond and me have strong opinions about how/whether
this problem should be fixed?

[rhettinger]

Closing since no one seems interested.

[jdunck]

I'm interested. I just had already said my peace and didn't know my
prior interest wasn't being counted. The patch uploaded by dmmartins
seemed good to me. I'm probably biased, since this bug affected me.

[mdickinson]

Just closed bpo-7323 as a duplicate of this one.

I think this issue is worth reopening: with the backport of the py3k
correctly rounded string <-> float conversions, there might now be a
reasonable way to rewrite Decimal.__hash__ so that it's consistent with
float.__hash__. Then we can make Decimal-to-float comparisons behave
correctly and clear up this mess.

I'd still be uncomfortable with allowing Decimal-to-float comparisons in
2.x but not in 3.x. Maybe they could be permitted in 3.x too?

[bertchughes]

Expressions like "Decimal('100.0') < .01" being silently wrong (True!) is *very* dangerous behavior, it should raise exception like any other Decimal-float operation, and hopefully will be back-ported to 2.7.

Eg: 3rd party module kinterbasdb, which provides access to Firebird database, returns floats from firebird large-int types (eg NUMERIC 18,2) in versions of kinrebasdb 3.2 or less, but in versions 3.3+ kinterbasdb retrieves large-int as type Decimal. This means if python/kinterbasdb users upgrade kinterbasdb they must be aware of this python bug, because all existing code must be inspected for "(retrieved Decimal value) compare (float)" statements, which before upgrade were Ok (retrieved float value) compare (float)) statements.

I'm new to this tracker, I hope this simply is added as an additional comment & squawk of dismay to the "float compared to decimal is silently incorrec" issue.

[mdickinson]

I'll try to find time to look at this again before 2.7 beta.

[mdickinson]

So here's the plan for trunk. The big question is: what should be done for py3k?

For trunk:

• all comparisons (==, !=, <=, >=, <, >) between floats and Decimals
  return the correct numerical result, as though the float were
  converted to a Decimal with no loss of precision. Like-signed float
  and Decimal infinities compare equal. float and Decimal nans compare
  unequal to everything else, as always.

• check whether anything special needs to be done to make sure that
  cmp also returns sensible results.

• fix Decimal.__hash__ so that when x == y for a float x and Decimal
  instance y, hash(x) == hash(y)

• the hash fix above is going to slow down hash computations, so
  consider caching hash values for Decimal instances (as a _hash
  attribute on the Decimal object itself). My gut feeling is that
  this probably isn't worth it, given that Decimal objects probably
  don't get hashed very often in normal use, but I'll do some timings
  to find out what the performance impact of the new hash is.

For py3k, the obvious options are:

(A) adopt the above changes, or

(B) keep Decimal and float non-comparable (as currently). In this case, a Decimal-to-float comparison in trunk should probably raise a DeprecationWarning. (Note that DeprecationWarnings are now silent by default, so such a warning wouldn't impact end-users much.)

[skrah]

I'm new to this thread, so I hope I didn't miss anything that has been
said already. I don't fully understand why TypeError cannot be raised
in 2.x. The 2.6 documentation for tp_richcompare says:

"If you want to implement a type for which only a limited set of comparisons makes sense (e.g. == and !=, but not < and friends), directly raise TypeError in the rich comparison function."

I just tried that in the 2.7 version of cdecimal, and it works well:

>>> from cdecimal import *
>>> Decimal(9) < 2.5
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: conversion from float to Decimal is not supported

[mdickinson]

Stefan: the problem is backwards compatibility. In 2.6 it's possible to sort a heterogeneous list that contains both Decimal instances and floats. The resulting order may not be particularly meaningful, but for some applications that doesn't matter.

If we make a Decimal-to-float comparison raise TypeError for 2.7 then sort will raise a TypeError where it used to work, so it's a potential code-breaking change. We could deprecate: raise a warning in 2.7 and make it a TypeError in 2.8, but since 2.8 currently seems unlikely to happen that would be a bit pointless.

[mdickinson]

Here's a patch:

• comparisons between a Decimal and float produce a result based
  on the numeric values of the Decimal and float

• change Decimal.__hash__ so that floats and Decimals with (exactly)
  equal value have the same hash. This preserves the rule that if
  two objects compare equal then they hash equal.

• add tests, and update documentation.

Still open: should this change be forward ported to py3k? If not, then these comparisons should produce a DeprecationWarning.

[mdickinson]

For anyone interested, there's an ongoing python-dev discussion about how to resolve this at

http://mail.python.org/pipermail/python-dev/2010-March/098437.html

[mdickinson]

Float-to-decimal comparisons have been fixed, and the Decimal hash function changed so that numerically equal Decimal and float instances hash equal, in r79583.

The idea of raising an exception for float<->Decimal comparisons was discarded partly for backwards compatibility reasons, and partly because having __eq__ raise an exception causes difficulties for sets and dicts: for example, if equality checks between floats and Decimals raised an exception then '{-1.0, Decimal(-3)}' would give a valid set (the two values have different hashes, so the Decimal.__eq__ method is never invoked), but '{-1.0, Decimal(-2)}' would raise an exception (because the two set elements have equal hashes, so Decimal.__eq__ is invoked in order to determine whether the two elements are equal or not).

(General principle: if x and y are hashable, x == y should never raise an exception.)

This is still only a partial fix: comparisons between Decimal and Fraction instances still behave oddly. This seems less likely to cause problems in real-life code, though. The changes needed to make Decimal <-> Fraction comparisons correct are too intrusive and not yet well tested enough to make it into 2.7. (See bpo-8188).

As discussed on python-dev, I'll forward port this to py3k; with any luck, py3k will also grow valid comparisons for Decimal <-> Fraction.

[mdickinson]

Merged to py3k in r79668.