classification
Title: _decimal difference with _pydecimal
Type: behavior Stage: resolved
Components: Versions: Python 3.7, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: andrewnester, arigo, mark.dickinson, rhettinger, skrah
Priority: normal Keywords:

Created on 2017-02-11 13:54 by arigo, last changed 2017-03-31 17:09 by skrah. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 65 andrewnester, 2017-02-13 10:57
PR 703 larry, 2017-03-17 21:00
PR 552 closed dstufft, 2017-03-31 16:36
Messages (14)
msg287600 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2017-02-11 13:54
A difference in behavior between _decimal and _pydecimal (it seems that _decimal is more consistent in this case):

    class X(Decimal):
        def __init__(self, a):
            print('__init__:', a)
    X.from_float(42.5)   # __init__: Decimal('42.5')

    X.from_float(42)     # with _pydecimal: __init__: 42
                         # with _decimal:   __init__: Decimal('42')
msg287612 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2017-02-11 16:38
I get the the same output, perhaps I misunderstand something here:

>>> from _decimal import *
>>> class X(Decimal):
...         def __init__(self, a):
...             print('__init__:', a)
... 
>>> X.from_float(42.5)
__init__: 42.5
Decimal('42.5')
>>> X.from_float(42)
__init__: 42
Decimal('42')
>>> 
>>> 
>>> from _pydecimal import *
>>> class X(Decimal):
...         def __init__(self, a):
...             print('__init__:', a)
... 
>>> X.from_float(42.5) 
__init__: 42.5
Decimal('42.5')
>>> X.from_float(42) 
__init__: 42
Decimal('42')
msg287615 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2017-02-11 18:06
Sorry!  It should be repr(a) inside the print.  Here is the fixed version:

    class X(Decimal):
        def __init__(self, a):
            print('__init__:', repr(a))
    X.from_float(42.5)   # __init__: Decimal('42.5')

    X.from_float(42)     # with _pydecimal: __init__: 42
                         # with _decimal:   __init__: Decimal('42')
msg287677 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-02-13 10:57
I've just added PR for this issue.
msg287680 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-13 11:58
Is there an observable difference in user behaviour if no subclassing of Decimal is involved? Or does this just affect Decimal subclasses?
msg287681 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-13 12:03
BTW, as a user, I'd expect `Decimal.from_float` to simply coerce its input to float if given an input that isn't actually a float in the first place; the fact that it has special handling in place for int inputs (and that that special handling takes the form of an isinstance check, rather than something more duck-typed) is surprising.
msg287682 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-02-13 12:04
actually, it's more related to subclassing, because the problem comes from the fact that before the fix __init__ method receives value as int not Decimal if int passed to from_float call. 
That's why only subclasses of Decimal can see difference in arguments received by __init__
msg287683 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-02-13 12:06
Agree about surprising behaviour but I guess it's better to fix it as other issue because it could break BC in some cases. At least it needs to be investigated.

For now I would like to stick with same behaviour for _decimal and _pydecimal
msg287687 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-13 13:05
[Andrew]

> it's better to fix it as other issue

No, it's better not to "fix" it at all. :-) It's one of those many behaviour changes where the answer to "Should we have done this differently in the first place" might possibly be "Yes" (and might also be "No"), but the answer to "Should we *change* it now" is a definite "No" (especially given that the "from_float" method is there almost entirely for backwards compatibility). I shouldn't have brought it up. Apologies.
msg287689 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-02-13 13:10
thanks for your notes, it's absolutely fine and I agree with you :)
msg287788 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-14 18:30
PR merged; thank you!

Unfortunately, just after I merged it I noticed that the Misc/NEWS entry was in the wrong section. I'll make a new PR to fix that, and close this issue once it's done.
msg287790 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-14 18:43
PR to fix the Misc/NEWS entry: https://github.com/python/cpython/pull/99
msg287793 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-14 19:21
All done. Closing. (In theory, as a bugfix, this could be backported to 3.5 and 3.6. In practice, I doubt it's worth the effort.)
msg290925 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2017-03-31 17:09
Does anyone know how to disable the spurious pull requests that keep coming in?
History
Date User Action Args
2017-03-31 17:09:42skrahsetmessages: + msg290925
2017-03-31 16:36:20dstufftsetpull_requests: + pull_request944
2017-03-17 21:00:35larrysetpull_requests: + pull_request610
2017-02-14 19:21:21mark.dickinsonsetstatus: open -> closed
resolution: fixed
messages: + msg287793

stage: resolved
2017-02-14 18:43:06mark.dickinsonsetmessages: + msg287790
2017-02-14 18:30:29mark.dickinsonsetmessages: + msg287788
2017-02-13 13:10:05andrewnestersetmessages: + msg287689
2017-02-13 13:05:31mark.dickinsonsetmessages: + msg287687
2017-02-13 12:06:05andrewnestersetmessages: + msg287683
2017-02-13 12:04:27andrewnestersetmessages: + msg287682
2017-02-13 12:03:38mark.dickinsonsetmessages: + msg287681
2017-02-13 11:58:26mark.dickinsonsetmessages: + msg287680
2017-02-13 10:57:06andrewnestersetnosy: + andrewnester

messages: + msg287677
pull_requests: + pull_request51
2017-02-11 18:06:29arigosetmessages: + msg287615
2017-02-11 16:38:00skrahsetmessages: + msg287612
2017-02-11 14:45:00skrahsetnosy: + rhettinger, mark.dickinson, skrah
2017-02-11 13:54:15arigocreate