classification
Title: float compared to decimal is silently incorrect.
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: adamtj, bas, bertchughes, facundobatista, jdunck, lorg, mark.dickinson, rhettinger, skrah
Priority: high Keywords: patch

Created on 2008-04-01 22:11 by jdunck, last changed 2010-04-03 11:45 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
decimal.patch dmmartins, 2008-04-02 02:55 always raise exception if 'other' can't be converted
float_decimal_comparisons.patch mark.dickinson, 2009-03-21 18:14
issue2531.patch mark.dickinson, 2010-03-14 11:52
Messages (35)
msg64827 - (view) Author: Jeremy Dunck (jdunck) Date: 2008-04-01 22:11
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)
msg64855 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-04-02 13:47
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
msg83691 - (view) Author: Imri Goldberg (lorg) Date: 2009-03-17 19:24
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.
msg83694 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-03-17 19:51
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.
msg83816 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-19 18:01
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?
msg83818 - (view) Author: Jeremy Dunck (jdunck) Date: 2009-03-19 18:15
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.
msg83935 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-21 11:08
> 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?
msg83939 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-03-21 17:38
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.
msg83940 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-03-21 17:42
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.
msg83941 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-21 17:47
What about Decimal <-> Fraction comparisons?
msg83942 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-03-21 17:58
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).
msg83944 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-21 18:14
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.
msg83948 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-21 19:03
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.
msg83950 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-21 20:37
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__.
msg83951 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-21 20:41
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)?
msg83952 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-03-21 20:53
I'll look at this more later today.
msg84433 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-03-29 21:42
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.
msg85099 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-04-01 21:30
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.
msg86053 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-04-17 02:45
Mark, any thoughts?  I would like to apply this patch for ordering
comparisons other than __eq__ and __ne__.
msg86058 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-17 08:37
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
msg86059 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-04-17 08:54
I'm not seeing the downside.  This gives better answers than it does now
but doesn't commit us to anything else.
msg86061 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-17 09:22
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 !=.
msg86123 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-18 16:36
Unassigning myself.

Does anyone beside Raymond and me have strong opinions about how/whether 
this problem should be fixed?
msg86438 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-04-24 23:03
Closing since no one seems interested.
msg86439 - (view) Author: Jeremy Dunck (jdunck) Date: 2009-04-24 23:14
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.
msg95263 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-11-14 22:37
Just closed issue 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?
msg97891 - (view) Author: Bert Hughes (bertchughes) Date: 2010-01-16 18:24
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.
msg97935 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-01-17 08:34
I'll try to find time to look at this again before 2.7 beta.
msg98215 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-01-24 11:23
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.)
msg98217 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2010-01-24 12:33
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
msg98218 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-01-24 12:37
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.
msg101053 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-03-14 11:52
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.
msg101231 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-03-17 15:32
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
msg102147 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-02 09:03
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 issue 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.
msg102242 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-04-03 11:45
Merged to py3k in r79668.
History
Date User Action Args
2010-04-03 11:45:02mark.dickinsonsetstatus: open -> closed
resolution: fixed
messages: + msg102242
2010-04-02 09:03:57mark.dickinsonsetmessages: + msg102147
versions: - Python 2.7
2010-03-17 15:32:19mark.dickinsonsetmessages: + msg101231
2010-03-14 11:52:33mark.dickinsonsetfiles: + issue2531.patch

messages: + msg101053
2010-03-12 00:41:06bassetnosy: + bas
2010-01-24 12:37:21mark.dickinsonsetmessages: + msg98218
2010-01-24 12:33:28skrahsetmessages: + msg98217
2010-01-24 11:23:26mark.dickinsonsetpriority: high

messages: + msg98215
versions: + Python 3.2, - Python 2.6
2010-01-18 09:20:10floxlinkissue7729 superseder
2010-01-17 08:34:44mark.dickinsonsetassignee: mark.dickinson
messages: + msg97935
2010-01-16 19:24:25skrahsetnosy: + skrah
2010-01-16 18:37:38floxsetversions: + Python 2.7
2010-01-16 18:24:36bertchughessetnosy: + bertchughes

messages: + msg97891
versions: + Python 2.6, - Python 2.7
2009-11-14 22:37:52mark.dickinsonsetnosy: + adamtj
2009-11-14 22:37:13mark.dickinsonsetstatus: closed -> open
resolution: wont fix -> (no value)
messages: + msg95263
2009-11-14 22:20:49mark.dickinsonlinkissue7323 superseder
2009-04-24 23:14:19jduncksetmessages: + msg86439
2009-04-24 23:03:02rhettingersetstatus: open -> closed
resolution: wont fix
messages: + msg86438
2009-04-18 16:37:04mark.dickinsonsetassignee: mark.dickinson -> (no value)
2009-04-18 16:36:55mark.dickinsonsetmessages: + msg86123
2009-04-17 09:22:50mark.dickinsonsetmessages: + msg86061
2009-04-17 08:54:32rhettingersetmessages: + msg86059
2009-04-17 08:37:08mark.dickinsonsetmessages: + msg86058
2009-04-17 02:45:58rhettingersetmessages: + msg86053
2009-04-01 21:34:34rhettingersetassignee: rhettinger -> mark.dickinson
2009-04-01 21:30:50rhettingersetmessages: + msg85099
2009-03-29 21:42:17mark.dickinsonsetkeywords: - easy

messages: + msg84433
2009-03-21 20:53:10rhettingersetmessages: + msg83952
2009-03-21 20:41:35mark.dickinsonsetmessages: + msg83951
2009-03-21 20:37:18mark.dickinsonsetmessages: + msg83950
2009-03-21 19:10:48rhettingersetassignee: mark.dickinson -> rhettinger
2009-03-21 19:03:37mark.dickinsonsetmessages: + msg83948
2009-03-21 18:14:04mark.dickinsonsetstatus: closed -> open
files: + float_decimal_comparisons.patch

versions: + Python 2.7, - Python 2.6, Python 2.5
keywords: + patch
resolution: rejected -> (no value)
messages: + msg83944
stage: patch review
2009-03-21 17:58:47rhettingersetmessages: + msg83942
2009-03-21 17:47:47mark.dickinsonsetmessages: + msg83941
2009-03-21 17:42:06rhettingersetmessages: + msg83940
2009-03-21 17:38:06rhettingersetmessages: + msg83939
2009-03-21 11:08:01mark.dickinsonsetmessages: + msg83935
2009-03-19 18:15:59jduncksetmessages: + msg83818
2009-03-19 18:01:32mark.dickinsonsetmessages: + msg83816
2009-03-17 19:51:19rhettingersetassignee: mark.dickinson

messages: + msg83694
nosy: + mark.dickinson, rhettinger
2009-03-17 19:24:26lorgsetnosy: + lorg
messages: + msg83691
2008-04-02 13:47:25facundobatistasetstatus: open -> closed
keywords: + easy, - patch
resolution: rejected
messages: + msg64855
nosy: + facundobatista
2008-04-02 03:08:42jduncksettype: behavior
2008-04-02 02:55:43dmmartinssetfiles: + decimal.patch
keywords: + patch
versions: + Python 2.6
2008-04-01 22:11:36jdunckcreate