_decimal difference with _pydecimal #73720

Activity

1. arigo commented on Feb 11, 2017

   

   arigomannequin

   on Feb 11, 2017

   MannequinAuthor

   More actions

   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')
2. arigo

   added

   3.7 (EOL)end of lifeend of life

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Feb 11, 2017
3. skrah commented on Feb 11, 2017

   

   skrahmannequin

   on Feb 11, 2017

   Mannequin

   More actions

   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')
4. arigo commented on Feb 11, 2017

   

   arigomannequin

   on Feb 11, 2017

   MannequinAuthor

   More actions

   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')
5. andrewnester commented on Feb 13, 2017

   

   andrewnestermannequin

   on Feb 13, 2017

   Mannequin

   More actions

   I've just added PR for this issue.
6. mdickinson commented on Feb 13, 2017

   

   mdickinson

   on Feb 13, 2017

   Member

   More actions

   Is there an observable difference in user behaviour if no subclassing of Decimal is involved? Or does this just affect Decimal subclasses?
7. mdickinson commented on Feb 13, 2017

   

   mdickinson

   on Feb 13, 2017

   Member

   More actions

   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.
8. andrewnester commented on Feb 13, 2017

   

   andrewnestermannequin

   on Feb 13, 2017

   Mannequin

   More actions

   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__
9. andrewnester commented on Feb 13, 2017

   

   andrewnestermannequin

   on Feb 13, 2017

   Mannequin

   More actions

   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
10. mdickinson commented on Feb 13, 2017

    

    mdickinson

    on Feb 13, 2017

    Member

    More actions

    [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.
11. andrewnester commented on Feb 13, 2017

    

    andrewnestermannequin

    on Feb 13, 2017

    Mannequin

    More actions

    thanks for your notes, it's absolutely fine and I agree with you :)
12. mdickinson commented on Feb 14, 2017

    

    mdickinson

    on Feb 14, 2017

    Member

    More actions

    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.
13. mdickinson commented on Feb 14, 2017

    

    mdickinson

    on Feb 14, 2017

    Member

    More actions

    PR to fix the Misc/NEWS entry: #99
14. mdickinson commented on Feb 14, 2017

    

    mdickinson

    on Feb 14, 2017

    Member

    More actions

    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.)
15. mdickinson

    closed this as completedon Feb 14, 2017
16. skrah commented on Mar 31, 2017

    

    skrahmannequin

    on Mar 31, 2017

    Mannequin

    More actions

    Does anyone know how to disable the spurious pull requests that keep coming in?
17. ezio-melotti

    transferred this issue fromon Apr 10, 2022