msg167284  (view) 
Author: Steven D'Aprano (steven.daprano) * 
Date: 20120803 02:54 
math.nan fails on some Decimal NANs. For example, while this works:
>>> import math
>>> from decimal import Decimal
>>> math.isnan(Decimal('nan'))
True
These both fail with ValueError:
math.isnan(Decimal('snan'))
math.isnan(Decimal('nan123'))
(Tested on Python 3.2 and 3.3.0a1)

msg167293  (view) 
Author: Mark Dickinson (mark.dickinson) * 
Date: 20120803 07:06 
Yep, Decimal.__float__ isn't too sophisticated. Probably it should convert all Decimal quiet NaNs (at least) to float NaNs, keeping the sign if possible but discarding any payload.
Not so sure about signaling NaNs, though; I think it would be fine for those to continue to raise ValueError (on the basis that doing pretty much anything with a signaling NaN should give an exception).

msg167300  (view) 
Author: Steven D'Aprano (steven.daprano) * 
Date: 20120803 09:06 
Attached is a patch for decimal.py, and test_decimal.py. I cannot provide a patch for the C decimal implementation, sorry.
Following Mark's suggestion, my patch keeps the sign but discards the payload for quiet NANs, and raises ValueError for signalling NANs.
(I'm ambivalent about signalling NANs raising ValueError, but I guess that's better than having snan silently converted to a quiet nan.)

msg167302  (view) 
Author: Stefan Krah (skrah) * 
Date: 20120803 09:24 
I think math.isnan('snan') probably should not raise. Decimal('snan').is_nan()
just returns true and I am under the impression that IEEE 754 specifies the
same. I have to admit though that I just consulted Wikipedia for the latter:
"The predicate isNaN(x) determines if a value is a NaN and never signals an
exception, even if x is a signaling NaN."

msg167309  (view) 
Author: Mark Dickinson (mark.dickinson) * 
Date: 20120803 12:06 
> Decimal('snan').is_nan()
> just returns true and I am under the impression that IEEE 754 specifies > the same.
Sure, but IEEE 754 also specifies that math.sqrt(<signalling nan>) should signal. Since both math.sqrt and math.isnan are going through __float__, we can't keep everyone happy here.
The question for me is really what __float__ should do. IEEE 754 doesn't help here, since it doesn't cover decimal floatingpoint <> binary floatingpoint conversions.

msg167316  (view) 
Author: Mark Dickinson (mark.dickinson) * 
Date: 20120803 12:26 
> IEEE 754 doesn't help here, since it doesn't cover decimal
> floatingpoint <> binary floatingpoint conversions.
OTOH, IEEE 754 *does* cover floatingpoint to int conversions (5.4.1, 5.8): those fall under 'generalcomputational operations', and as such should signal when given an sNaN (6.2: "Signaling NaNs shall be reserved operands that, under default exception handling, signal the invalid operation exception (see 7.2) for every generalcomputational and signalingcomputational operation except for the conversions described in 5.12. For nondefault treatment, see 8."). It feels to me as though decimal > binary conversions should follow the same pattern.

msg167323  (view) 
Author: Steven D'Aprano (steven.daprano) * 
Date: 20120803 13:03 
On 03/08/12 22:06, Mark Dickinson wrote:
>
>> Decimal('snan').is_nan() just returns true and I am under the impression
>> that IEEE 754 specifies the same.
>
> Sure, but IEEE 754 also specifies that math.sqrt(<signalling nan>) should
> signal. Since both math.sqrt and math.isnan are going through __float__,
> we can't keep everyone happy here.
Is it necessarily a given that math.isnan *must* go through __float__? If it
were written in Python, it would be a simple matter of including
if isinstance(x, Decimal) and x.isnan(): return True
before the conversion to float. By I have no idea whether that is practical in
the math module.
> The question for me is really what __float__ should do. IEEE 754 doesn't
> help here, since it doesn't cover decimal floatingpoint<> binary
> floatingpoint conversions.
Until such time that floats officially support snans, I think ValueError is
the right behaviour.

msg167326  (view) 
Author: Mark Dickinson (mark.dickinson) * 
Date: 20120803 13:33 
> before the conversion to float. By I have no idea whether that is
> practical in the math module.
That's a much bigger discussion: as it is, most of the math module functions just provide simple wrappers around the system math library, which deals purely with floats. *Some* of the math module functions have been made more generic, like floor and ceil (which look for special __floor__ and __ceil__ methods), but isnan isn't one of those: like most of the math module functions, it simply converts whatever it's given to float, then passes it on to the system library and returns whatever it gets back.
Changing that would be feature request targeted at 3.4 or later; I'm not saying that it shouldn't be considered, but it doesn't belong in this issue.

msg167331  (view) 
Author: Antoine Pitrou (pitrou) * 
Date: 20120803 14:26 
Why not add a is_nan() method to float numbers instead?

msg167346  (view) 
Author: Stefan Krah (skrah) * 
Date: 20120803 19:41 
> OTOH, IEEE 754 *does* cover floatingpoint to int conversions (5.4.1, 5.8): those fall under 'generalcomputational operations', and as such should signal when given an sNaN.
That sounds good. Let's keep the ValueError then. We could consider
InvalidOperation like with Decimal('snan').to_integral(), but that's
probably confusing in the context of the math module.

msg167347  (view) 
Author: Stefan Krah (skrah) * 
Date: 20120803 19:47 
> Why not add a is_nan() method to float numbers instead?
Do you mean replacing math.isnan(x) by x.is_nan() to avoid the issue
altogether? I'm not sure that's possible given that math just wraps
the C library.

msg167348  (view) 
Author: Antoine Pitrou (pitrou) * 
Date: 20120803 19:49 
> > Why not add a is_nan() method to float numbers instead?
>
> Do you mean replacing math.isnan(x) by x.is_nan() to avoid the issue
> altogether? I'm not sure that's possible given that math just wraps
> the C library.
Yup. By calling x.is_nan() you would by construction get an
implementation that's correct for x's type. If x is a float, it would
obviously reuse math.isnan() (or have a similar implementation).

msg167422  (view) 
Author: Mark Dickinson (mark.dickinson) * 
Date: 20120804 17:45 
> Why not add a is_nan() method to float numbers instead?
That could work. The duplication of float.is_nan and math.isnan (not to mention the different spellings) would be a bit ugly, but perhaps worth it. It would make sense to add float.is_infinite and (possibly) float.is_finite methods at the same time.
Looks like we've got two separate issues here, that should probably be split into two separate bug reports. The first issue is that Decimal.__float__ is braindead when it comes to NaNs with payloads; I consider that a clear bug, and Steven's patch fixes it nicely for the Python version of decimal. The second has to do with finding a nice typeagnostic way of determing whether something is a NaNanyone mind if I open a separate issue for this?
W.r.t. the first issue: Steven, thanks for the patch; looks fine to me at first glance.
Two questions: (1) What would you think about raising ValueError explicitly for the signaling NaN case rather than falling back to the ValueError coming from the stringtofloat conversion. I think the intentions of the code would be a little clearer that way; and we get to choose a more informative error message that way, too. (2) Should we apply the fix to 2.7 and/or 3.2 as well?
I'll look at extending Steven's fix to the cdecimal code, unless Stefan really wants to do it (which would be fine with me :).

msg167428  (view) 
Author: Stefan Krah (skrah) * 
Date: 20120804 18:52 
Mark Dickinson <report@bugs.python.org> wrote:
> Looks like we've got two separate issues here, that should probably be
> split into two separate bug reports. The first issue is that
> Decimal.__float__ is braindead when it comes to NaNs with payloads;
> I consider that a clear bug, and Steven's patch fixes it nicely for
> the Python version of decimal.
If we are viewing the whole issue in terms of decimal > float conversion,
I'm not so sure anymore: Not all Decimal payloads have a meaning for floats
(and payloads get lost in the conversion).
On the other hand it doesn't matter since I doubt anyone is using payloads. :)
> The second has to do with finding a nice typeagnostic way of determing
> whether something is a NaNanyone mind if I open a separate issue for this?
Yes, that should probably be another issue.
> Two questions: (1) What would you think about raising ValueError explicitly
> for the signaling NaN case rather than falling back to the ValueError coming
> from the stringtofloat conversion.
If we use your latest rationale for raising in case of Decimal('snan').__float__(),
I think that indeed __float__() should raise like Decimal.to_integral() does
for example.
If we are aiming for sNaN support in floats in the long term and at some point
allow carrying over sNaNs from decimal to float, then perhaps the error message
could say that sNaN conversion is currently not supported.
> (2) Should we apply the fix to 2.7 and/or 3.2 as well?
Yes, I think so.
> I'll look at extending Steven's fix to the cdecimal code, unless Stefan
> really wants to do it (which would be fine with me :).
Please go ahead! For this year, I've seen more than enough of _decimal.c
already. :)

msg167469  (view) 
Author: Steven D'Aprano (steven.daprano) * 
Date: 20120805 01:02 
On 05/08/12 03:45, Mark Dickinson wrote:
> It would make sense to add float.is_infinite and (possibly) float.is_finite
> methods at the same time.
If you don't add is_finite, you know someone is going to express surprise that
it wasn't already done. Just as happened with math.isfinite :)
http://bugs.python.org/issue9165#msg109326
> The second has to do with finding a nice
> typeagnostic way of determing whether something is a NaNanyone mind if
> I open a separate issue for this?
Please do.
> Two questions: (1) What would you think about raising ValueError
> explicitly for the signaling NaN case [...] (2) Should we apply
> the fix to 2.7 and/or 3.2 as well?
Agree to both. I think this counts as a bug report and not a new feature.
> I'll look at extending Steven's fix to the cdecimal code
Thank you :)

msg167675  (view) 
Author: Raymond Hettinger (rhettinger) * 
Date: 20120808 08:06 
0.5 on making the math module functions aware of decimals.
The math module was originally conceived as thin wrapper around the C math library. Subsequently, it has had feature creep (I'm guilty of putting the integer factorial method in this module). I don't think further creep and loss of focus is warranted.
Making some functions decimal aware and others not is probably not a good idea.
Also, if someone is using decimals, they are better of using the methods supplied in that module (those have at least passed the huge battery of tests for compliance with the spec).

msg168562  (view) 
Author: Mark Dickinson (mark.dickinson) * 
Date: 20120819 11:52 
Here's an updated patch that extends Steven's fix to the C code.

msg168574  (view) 
Author: Stefan Krah (skrah) * 
Date: 20120819 13:37 
The patch looks good in every detail. +1 for committing.

msg169059  (view) 
Author: Roundup Robot (pythondev) 
Date: 20120824 17:53 
New changeset 49014c59b31f by Mark Dickinson in branch 'default':
Issue #15544: Fix Decimal.__float__ to work with payloadcarrying NaNs.
http://hg.python.org/cpython/rev/49014c59b31f

msg169067  (view) 
Author: Roundup Robot (pythondev) 
Date: 20120824 18:40 
New changeset a931e44ffbe1 by Mark Dickinson in branch '3.2':
Issue #15544: Fix Decimal.__float__ to work with payloadcarrying NaNs.
http://hg.python.org/cpython/rev/a931e44ffbe1

msg169072  (view) 
Author: Roundup Robot (pythondev) 
Date: 20120824 19:06 
New changeset 5dd5f824428c by Mark Dickinson in branch '2.7':
Issue #15544: Fix Decimal.__float__ to work with payloadcarrying NaNs.
http://hg.python.org/cpython/rev/5dd5f824428c

msg169074  (view) 
Author: Mark Dickinson (mark.dickinson) * 
Date: 20120824 19:12 
Fixed. (I managed to mess up the commit to 3.2 and break all the buildbots :(. I think it's okay now.)
Thanks Steven for the report and patch! (And thanks Stefan for reviewing.)
