Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-119613: deprecate Py_IS_NAN/INFINITY and Py_IS_FINITE #119701

Merged
merged 11 commits into from
May 29, 2024

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented May 29, 2024

Doc/whatsnew/3.14.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.14.rst Outdated Show resolved Hide resolved
skirpichev and others added 3 commits May 29, 2024 06:04
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@@ -193,6 +193,10 @@ Porting to Python 3.14
Deprecated
----------

* Deprecate ``Py_IS_NAN``, ``Py_IS_INFINITY`` and ``Py_IS_FINITE`` macros,
use instead ``isnan``, ``isinf`` and ``isfinite`` available from ``<math.h>``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use instead ``isnan``, ``isinf`` and ``isfinite`` available from ``<math.h>``
use instead :c:func:`!isnan`, :c:func:`!isinf` and :c:func:`!isfinite` available from :file:`math.h`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, these are usually macros, e.g.:
https://en.cppreference.com/w/c/numeric/math/isinf

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure; but this is about the markup :)

Doc/whatsnew/3.14.rst Outdated Show resolved Hide resolved
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to soft deprecate these macros, with a link to the glossary in thr What's New and Changelog entries (:term: syntax).

Include/pymath.h Outdated Show resolved Hide resolved
Include/pymath.h Show resolved Hide resolved
skirpichev and others added 2 commits May 29, 2024 11:14
Co-authored-by: Victor Stinner <vstinner@python.org>
Include/pymath.h Outdated Show resolved Hide resolved
Include/pymath.h Outdated Show resolved Hide resolved
Include/pymath.h Outdated Show resolved Hide resolved
vstinner and others added 3 commits May 29, 2024 11:45
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. IMO a soft deprecation is enough. Keeping these macros around is not a major maintenance burden.

If PEP 743 is accepted, maybe we can provide a way to remove this function with an opt-in option.

@vstinner vstinner enabled auto-merge (squash) May 29, 2024 09:48
@vstinner
Copy link
Member

Thanks for the updates @skirpichev, I enabled automerge.

@skirpichev: If you are interested, there are other macros which might be worth it to soft deprecate such as Py_MEMCPY(). I removed this macro in the limited C API version 3.11:

#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 < 0x030b0000
#  define Py_MEMCPY memcpy
#endif

@skirpichev
Copy link
Member Author

@vstinner, I think automerge wont work due to canceled test (I saw similar failure in #118000).

@vstinner vstinner merged commit 0cdc5c8 into python:main May 29, 2024
37 of 38 checks passed
@skirpichev skirpichev deleted the deprecate-py-is-nan-p2-119613 branch May 29, 2024 10:52
@skirpichev
Copy link
Member Author

@vstinner, we can deprecate also Py_INFINITY, Py_NAN and Py_HUGE_VAL. For the last one, probably we could also update code to use INFINITY instead. What do you think?

@vstinner
Copy link
Member

vstinner commented Jun 4, 2024

What do you think?

You can start by avoiding these macros in Python code base and see how it goes.

Py_INFINITY and Py_NAN add a (double) cast.

@skirpichev
Copy link
Member Author

Py_INFINITY and Py_NAN add a (double) cast.

Hmm, from C11 Sec 7.2: "The macro INFINITY expands to a constant expression of type float representing positive or unsigned infinity, if available [...] The macro NAN is defined if and only if the implementation supports quiet NaNs for the float type. It expands to a constant expression of type float representing a quiet NaN." (c)

As now build requirements include "Support for IEEE 754 floating point numbers and floating point Not-a-Number (NaN).", I think nowadays this type cast is useless.

Yet, anyway, replacing (with a soft deprecation) Py_HUGE_VAL by Py_INFINITY - probably does make sense. I was mentioned in #104202 (comment). One obvious pro: current using both Py_HUGE_VAL and Py_INFINITY may confuse readers.

noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants