classification
Title: Add float.is_finite is_nan is_infinite to match Decimal methods
Type: enhancement Stage: resolved
Components: Versions: Python 3.7
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: martin.panter Nosy List: Mariatta, christian.heimes, corona10, facundobatista, mark.dickinson, martin.panter, rhettinger, serhiy.storchaka, steven.daprano, vstinner
Priority: normal Keywords: patch

Created on 2013-08-26 15:59 by steven.daprano, last changed 2020-01-22 04:16 by corona10. This issue is now closed.

Files
File name Uploaded Description Edit
float_is.patch christian.heimes, 2013-10-13 11:31 review
issue18842.patch christian.heimes, 2013-10-13 12:05 review
rm-finite.patch martin.panter, 2017-01-30 00:06 review
Messages (19)
msg196219 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2013-10-13 13:49
Fraction? complex? Complex? Integral? Number?
msg199705 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2017-01-29 11:04
Of course, somehow I missed that
msg286446 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2020-01-22 04:16:51corona10setstatus: open -> closed

nosy: + corona10
messages: + msg360447

resolution: rejected
stage: patch review -> resolved
2017-02-03 08:05:47vstinnersetnosy: + vstinner
2017-02-03 05:24:29Mariattasetassignee: Mariatta -> martin.panter
messages: + msg286842
2017-01-31 04:01:30rhettingersetassignee: Mariatta

messages: + msg286515
nosy: + Mariatta
2017-01-30 09:04:25mark.dickinsonsetmessages: + msg286476
2017-01-30 00:06:12martin.pantersetfiles: + rm-finite.patch

messages: + msg286462
versions: + Python 3.7, - Python 3.4
2017-01-29 20:53:03serhiy.storchakasetmessages: + msg286449
2017-01-29 19:31:33rhettingersetmessages: + msg286447
2017-01-29 18:02:42mark.dickinsonsetmessages: + msg286446
2017-01-29 11:04:15martin.pantersetmessages: + msg286442
2017-01-29 11:01:18steven.dapranosetmessages: + msg286441
2017-01-29 08:23:05martin.pantersetnosy: + martin.panter
messages: + msg286434
2014-10-14 16:23:43skrahsetnosy: - skrah
2013-11-17 07:14:10steven.dapranosetmessages: + msg203122
2013-11-16 16:33:36serhiy.storchakasetmessages: + msg203061
2013-10-13 13:56:48christian.heimessetmessages: + msg199705
2013-10-13 13:49:24serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg199704
2013-10-13 12:05:23christian.heimessetfiles: + issue18842.patch

messages: + msg199699
stage: patch review
2013-10-13 11:31:14christian.heimessetfiles: + float_is.patch

nosy: + christian.heimes
messages: + msg199696

keywords: + patch
2013-10-13 09:37:52mark.dickinsonsetmessages: + msg199680
2013-08-26 17:46:29serhiy.storchakasetnosy: + rhettinger, facundobatista, mark.dickinson, skrah
2013-08-26 15:59:17steven.dapranocreate