This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Decimal.from_float works incorrectly for non-binary floats
Type: behavior Stage: resolved
Components: Extension Modules, Library (Lib) Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: facundobatista, mark.dickinson, rhettinger, serhiy.storchaka, skrah
Priority: normal Keywords:

Created on 2016-05-08 06:45 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Messages (8)
msg265116 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-08 06:45
Decimal.from_float() works correctly only if the denominator of as_integer_ratio() of argument is the power of two. Example:

>>> from decimal import Decimal
>>> class DecimalFloat(float):
...     def as_integer_ratio(self):
...         return Decimal(str(self)).as_integer_ratio()
...     def __abs__(self):
...         return self
... 
>>> DecimalFloat(1.2).as_integer_ratio()
(6, 5)
>>> Decimal.from_float(DecimalFloat(1.2))
Decimal('1.50')
msg265118 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-05-08 07:33
True. Though having a subclass of float that overrides as_integer_ratio seems a fairly unlikely use-case. We could add a check for subclasses of float that the denominator is a power of 2 (using the usual trick: a positive integer n is a power of 2 if and only if `n & (n-1)` is zero) and raise a suitable error otherwise.

I doubt it's worth trying to support arbitrary return values from as_integer_ratio. Note that by overriding as_integer_ratio, you're breaking its "contract": the docs say

> Return a pair of integers, whose ratio is exactly equal to the original
> float and with a positive denominator.

and you've lost that "exactly equal". I think it's reasonably to get wrong results or an exception in that case.
msg265123 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-05-08 10:37
It seems to work correctly here for non-binary floats:

>>> from _pydecimal import Decimal
>>> Decimal.from_float(Decimal("1.2"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.6/_pydecimal.py", line 739, in from_float
    raise TypeError("argument must be int or float.")
TypeError: argument must be int or float.


I think we should not support strange inheritance hierarchies that
break the expected return values of parent class methods.
msg265138 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-08 13:25
Once I was going to propose to make as_integer_ratio() returning a pair (n, d) with minimal positive d that n/d (as float) is exactly equal to the original float. I think this is legitimate implementation and doesn't contradict the documentation.

I see following ways to resolve this issue:

1. Ignore it.

2. Ignore methods overriding:
2a) either convert float subclass to exact float with known behavior;
2b) or directly call static float.__abs__() and float.as_integer_ratio() instead of resolving dynamic methods.

3. Check that the denominator is a power of 2 and raise error otherwise.

4. Implement different different algorithm. Returns decimal n / 10**k with minimal k that after converting to float is exactly equal to the original float.

Most of these solutions will automatically fix issue26974, so it doesn't need special fix.
msg265140 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-05-08 13:39
As Mark hinted at, many people would say there is no issue at all.
Subclassing like that often breaks the Liskov Substitution Principle.

For more information, see e.g.:

  http://okmij.org/ftp/Computation/Subtyping/


"Alas, LSP when considered from an OOP point of view is undecidable. You cannot count on a compiler for help in pointing out an error. You cannot rely on regression tests either. It's manual work -- you have to see the problem"
msg265142 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-05-08 14:32
> I see following ways to resolve this issue:

I don't think 4. is an option: for input of exact type float, from_float should continue to produce the exact value of the float, rather than an approximation to that exact value. There's code out there that depends on this behaviour. Options 1 through 3 all seem reasonable to me.
msg269029 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-06-21 21:31
I vote for ignoring this and calling it not-a-bug.  In a way, it is no more interesting than a dict like object defining __hash__ to return a random number.

The intention for from_float() is to convert binary floats.  It was not meant to be generalized to handle arbitrary fractions.  I concur with Mark's comment, "Note that by overriding as_integer_ratio, you're breaking its "contract"".
msg269030 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2016-06-21 21:58
Let's close it then.
History
Date User Action Args
2022-04-11 14:58:30adminsetgithub: 71162
2016-06-21 21:58:01skrahsetstatus: open -> closed
resolution: not a bug
messages: + msg269030

stage: resolved
2016-06-21 21:31:24rhettingersetmessages: + msg269029
2016-05-08 14:32:12mark.dickinsonsetmessages: + msg265142
2016-05-08 13:39:06skrahsetmessages: + msg265140
2016-05-08 13:25:20serhiy.storchakasetmessages: + msg265138
2016-05-08 10:37:40skrahsetmessages: + msg265123
2016-05-08 07:33:26mark.dickinsonsetmessages: + msg265118
2016-05-08 06:45:45serhiy.storchakacreate