msg196219 - (view) |
Author: Steven D'Aprano (steven.daprano) * |
Date: 2013-08-26 15:59 |
On issue 15544 Mark Dickinson suggested adding methods to float to match methods on Decimal, giving type-agnostic ways of testing real numbers that don't rely on converting to float. I don't see any sign that Mark raised a feature request, so I'm taking the liberty of doing so myself.
Note that the math.is* functions convert to float first, which means that they behave differently. Example: math.isnan(Decimal('sNAN')) raises ValueError, rather than returning True.
float.is_nan
float.is_infinity
float.is_finite
would mirror the spelling of Decimal methods, rather than the math module. As float doesn't support signalling NANs, there probably isn't any need to include is_snan and is_qnan.
For what it's worth, I have code that would use this. I currently write something like:
if isinstance(x, Decimal) and x.is_nan() or math.isnan(x): ...
in order to prevent triggering the ValueError on signalling NANs.
|
msg199680 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-10-13 09:37 |
+1 from me. You want `float.is_infinite` rather than `float.is_infinity`. `is_signed` is another one that may be worth considering.
|
msg199696 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2013-10-13 11:31 |
The code is already there so the patch is really small.
http://hg.python.org/cpython/annotate/5bc7b20dc04a/Objects/floatobject.c#l1046
I love my time machine. *g*
|
msg199699 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2013-10-13 12:05 |
Here is a longer patch that also adds the methods to int and numbers.Real. It comes with tests and doc updates, too.
|
msg199704 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-10-13 13:49 |
Fraction? complex? Complex? Integral? Number?
|
msg199705 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2013-10-13 13:56 |
Pardon?
The methods could be added to complex, too. cmath implements the methods as:
is_finite:
Py_IS_FINITE(z.real) && Py_IS_FINITE(z.imag)
is_infinite:
Py_IS_INFINITY(z.real) || Py_IS_INFINITY(z.imag)
is_nan:
Py_IS_NAN(z.real) || Py_IS_NAN(z.imag)
For numbers.Real: We can't make the methods abstractmethods because it would be an incompatible change.
|
msg203061 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-11-16 16:33 |
Should we add these methods to other concrete Number subclasses (as Fraction and complex)?
|
msg203122 - (view) |
Author: Steven D'Aprano (steven.daprano) * |
Date: 2013-11-17 07:14 |
On Sat, Nov 16, 2013 at 04:33:36PM +0000, Serhiy Storchaka wrote:
>
> Should we add these methods to other concrete Number subclasses (as Fraction and complex)?
Seems like a good idea to me. Is it worth making them part of the Number
ABC, or is that too much of a change?
|
msg286434 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2017-01-29 08:23 |
FWIW, here is an attempt to add Argument Clinic to the Objects/floatobject.c and Objects/longobject.c implementations:
https://bugs.python.org/file33943/issue20185_conglomerate_v4.diff
https://bugs.python.org/file33989/clinic_longobject_v3.patch
If the methods are rejected, the dead code should be removed to avoid wasting people’s time.
Why do you name the methods is_finite() etc with underscores, when the existing methods math.isfinite() etc do not have underscores? Seems it would add unnecessary confusion.
|
msg286441 - (view) |
Author: Steven D'Aprano (steven.daprano) * |
Date: 2017-01-29 11:01 |
On Sun, Jan 29, 2017 at 08:23:05AM +0000, Martin Panter wrote:
> Why do you name the methods is_finite() etc with underscores, when the
> existing methods math.isfinite() etc do not have underscores? Seems it
> would add unnecessary confusion.
The idea is to enable duck-typing between float and Decimal. Instead of:
if isinstance(x, float):
math.isfinite(x)
else:
x.is_finite()
we can just say
x.is_finite()
on both floats and decimals.
|
msg286442 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2017-01-29 11:04 |
Of course, somehow I missed that
|
msg286446 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2017-01-29 18:02 |
@Martin: the dead code should definitely be removed from floatobject.c and longobject.c. (Though that's kinda independent of this issue.)
|
msg286447 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2017-01-29 19:31 |
I would like to caution against expansion of core APIs in cases where we already have a way to do it. In almost every corner, Python's APIs are growing, resulting in the language becoming massive, where very few people on the planet can claim to know what is in it and it is becoming much harder to teach as it gets bigger.
Also, we should place very little value on anecdotal evidence from one core developer who says he once wrote a single line of code that would have benefitted from a single part of the proposed changes. In general, core devs tend to write much more generic code than end users, so our needs are often atypical and do not reflect user needs (in this case, no non-core-dev numeric user has ever requested this behavior or had a real use case that couldn't be met by the existing APIs). The bar for new APIs is much higher than "I wrote a line of code once".
We should be looking for APIs additions that significantly reduce complexity or solve problems that can't easily be met with existing APIs. Minor expansions create long term maintenance burdens, increase the size of docs making them less usable, cause other implementations to have to follow suit, cause subclassers and users of the numeric module to have to implement additional methods, and make Python more difficult to learn. There is a reason that the zen expresses a preference for only one way to do it.
Replicating parts the decimal API should always be suspect as well. In general, that module is much more difficult to use and learn than regular floats. Many parts of the API are simply worthless and are there only because they were part of the spec. We have no evidence that any of these proposed methods are actually being used and are of real benefit to real users.
Also, remember that Decimal is not registered as a Real for a reason. Guido does not consider them to be interoperable with binary floats.
|
msg286449 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-01-29 20:53 |
I concur with Raymond. This expands the API too much. Not just the float API, but the API of all numeric classes, including third-party classes. And since the existence of additional method in third-party classes (e.g. NumPy classes) can't be guarantied, this couldn't help in the case of msg286441.
The less harmful way is making math.isfinite() and like supporting non-float numeric types. Always return False for integer types and fall back to the is_finite() method. But even this can be too expensive.
|
msg286462 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2017-01-30 00:06 |
While I don’t have much opinion either way, here is a patch to remove the existing dead code should that be the eventual outcome.
If the default implementations in the base class deferred to math.isfinite() etc, that may help with compatibility.
|
msg286476 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2017-01-30 09:04 |
rm-finite.patch LGTM.
Once that's merged, let's close this issue as rejected. If decimal is ever added as a first-class type (literal support, fixed-width decimal128 or decimal64, etc.), we may want to reconsider, but that day is probably a long way off.
|
msg286515 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2017-01-31 04:01 |
Mariatta, please review the rm-finite patch. We're already agreed that the removal makes sense. What you're checking for is whether patch is complete (that there aren't any dangling references to the removed code and that it doesn't remove too much). Also, try out the patch and run the test suite in in a debug mode.
Whether you find any issues or are ready to bless the patch, make a comment in the tracker and assign back to Martin (the core dev who posts the patch is usually the one to apply it). After merging, he will mark the class as closed/rejected.
|
msg286842 - (view) |
Author: Mariatta (Mariatta) * |
Date: 2017-02-03 05:24 |
Martin,
I reviewed the patch (rm-finite.patch) and ran the tests. The patch no longer applies cleanly to the default branch, but otherwise looks good.
Assigning this back to you as per Raymond's suggestion.
Thanks :)
|
msg360447 - (view) |
Author: Dong-hee Na (corona10) * |
Date: 2020-01-22 04:16 |
rm-finite.patch was applied by bpo-39415.
> Once that's merged, let's close this issue as rejected.
I close this issue to a rejected state as discussed above.
Thank you to all who participated in this discussion.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:50 | admin | set | github: 63042 |
2020-01-22 04:16:51 | corona10 | set | status: open -> closed
nosy:
+ corona10 messages:
+ msg360447
resolution: rejected stage: patch review -> resolved |
2017-02-03 08:05:47 | vstinner | set | nosy:
+ vstinner
|
2017-02-03 05:24:29 | Mariatta | set | assignee: Mariatta -> martin.panter messages:
+ msg286842 |
2017-01-31 04:01:30 | rhettinger | set | assignee: Mariatta
messages:
+ msg286515 nosy:
+ Mariatta |
2017-01-30 09:04:25 | mark.dickinson | set | messages:
+ msg286476 |
2017-01-30 00:06:12 | martin.panter | set | files:
+ rm-finite.patch
messages:
+ msg286462 versions:
+ Python 3.7, - Python 3.4 |
2017-01-29 20:53:03 | serhiy.storchaka | set | messages:
+ msg286449 |
2017-01-29 19:31:33 | rhettinger | set | messages:
+ msg286447 |
2017-01-29 18:02:42 | mark.dickinson | set | messages:
+ msg286446 |
2017-01-29 11:04:15 | martin.panter | set | messages:
+ msg286442 |
2017-01-29 11:01:18 | steven.daprano | set | messages:
+ msg286441 |
2017-01-29 08:23:05 | martin.panter | set | nosy:
+ martin.panter messages:
+ msg286434
|
2014-10-14 16:23:43 | skrah | set | nosy:
- skrah
|
2013-11-17 07:14:10 | steven.daprano | set | messages:
+ msg203122 |
2013-11-16 16:33:36 | serhiy.storchaka | set | messages:
+ msg203061 |
2013-10-13 13:56:48 | christian.heimes | set | messages:
+ msg199705 |
2013-10-13 13:49:24 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg199704
|
2013-10-13 12:05:23 | christian.heimes | set | files:
+ issue18842.patch
messages:
+ msg199699 stage: patch review |
2013-10-13 11:31:14 | christian.heimes | set | files:
+ float_is.patch
nosy:
+ christian.heimes messages:
+ msg199696
keywords:
+ patch |
2013-10-13 09:37:52 | mark.dickinson | set | messages:
+ msg199680 |
2013-08-26 17:46:29 | serhiy.storchaka | set | nosy:
+ rhettinger, facundobatista, mark.dickinson, skrah
|
2013-08-26 15:59:17 | steven.daprano | create | |