FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-153144: Avoid checking `errno` for `atan2` by hpkfft · Pull Request #153148 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-153144: Avoid checking errno for atan2 #153148

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.

Sign up for GitHub

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

Open
hpkfft wants to merge 21 commits into
base: main
Choose a base branch
Could not load branches
Branch not found: {{ refName }}

Are you sure you want to change the base?

Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
from
Conversation 69 Commits 21 Checks 50 Files changed
Open

gh-153144: Avoid checking errno for atan2 #153148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
503fd4a
gh-153144: Avoid checking `errno` for `atan2`
hpkfft Jul 5, 2026
c9c51ad
gh-153144: Added NEWS entry
hpkfft Jul 20, 2026
287e6ee
gh-153144: Modify comments as requested in review
hpkfft Jul 21, 2026
cbe759a
Merge branch 'main' into atan2
hpkfft Aug 1, 2026
91257f2
Merge branch 'main' into atan2
hpkfft Aug 13, 2026
25ec8e2
Add comments as requested in review
hpkfft Aug 14, 2026
43f7398
Revert gh-146402 and update NEWS entry
hpkfft Aug 14, 2026
5b6d005
Merge branch 'main' into atan2
skirpichev Aug 30, 2026
ead2720
Make NEWS entry a little shorter.
hpkfft Aug 30, 2026
d546e81
The copysign function does not need to check errno.
hpkfft Aug 30, 2026
f4ed052
Merge branch 'main' into atan2
hpkfft Sep 8, 2026
82448e1
Add cmath tests and update NEWS entry
hpkfft Sep 8, 2026
288fb8b
Merge branch 'main' into atan2
hpkfft Sep 18, 2026
be0ed87
Expand comments to explain why we ignore errno
hpkfft Sep 18, 2026
b991892
Merge branch 'main' into atan2
hpkfft Sep 18, 2026
72842e1
Merge branch 'main' into atan2
hpkfft Oct 1, 2026
11c56c4
Merge branch 'main' into atan2
hpkfft Oct 3, 2026
6a92490
Change as requested in review
hpkfft Oct 3, 2026
6a48f02
Merge branch 'main' into atan2
hpkfft Oct 4, 2026
8594452
Add tests for math.atan2() and update NEWS
hpkfft Oct 4, 2026
527afd1
Remove decorative `*` from comment in cmathmodule
hpkfft Oct 4, 2026
File filter

Filter by extension

Filter by extension .c  (2) .py  (2) .rst  (2) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
12 changes: 10 additions & 2 deletions Lib/test/test_cmath.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,6 @@ def polar_with_errno_set(z):
_testcapi.set_errno(0)
self.check_polar(polar_with_errno_set)

@unittest.skipIf(sys.platform.startswith("sunos"),
"skipping, see gh-138573")
def test_phase(self):
self.assertAlmostEqual(phase(0), 0.)
self.assertAlmostEqual(phase(1.), 0.)
Expand All @@ -423,6 +421,16 @@ def test_phase(self):
self.assertEqual(phase(complex(-0.0, 0.0)), pi)
self.assertEqual(phase(complex(-0.0, -0.0)), -pi)

# overflow and underflow of imag/real
self.assertEqual(phase(complex(1E300, 1E-320)), 0.0)
self.assertEqual(phase(complex(1E300, -1E-320)), -0.0)
self.assertAlmostEqual(phase(complex(1E-320, 1E300)), pi/2)
self.assertAlmostEqual(phase(complex(-1E-320, 1E300)), pi/2)
self.assertAlmostEqual(phase(complex(-1E300, 1E-320)), pi)
self.assertAlmostEqual(phase(complex(-1E300, -1E-320)), -pi)
self.assertAlmostEqual(phase(complex(1E-320, -1E300)), -pi/2)
self.assertAlmostEqual(phase(complex(-1E-320, -1E300)), -pi/2)

# infinities
self.assertAlmostEqual(phase(complex(-INF, -0.0)), -pi)
self.assertAlmostEqual(phase(complex(-INF, -2.3)), -pi)
Expand Down
11 changes: 9 additions & 2 deletions Lib/test/test_math.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,6 @@ def testAtanh(self):
self.assertRaises(ValueError, math.atanh, NINF)
self.assertTrue(math.isnan(math.atanh(NAN)))

@unittest.skipIf(sys.platform.startswith("sunos"),
"skipping, see gh-138573")
def testAtan2(self):
self.assertRaises(TypeError, math.atan2)
self.ftest('atan2(-1, 0)', math.atan2(-1, 0), -math.pi/2)
Expand Down Expand Up @@ -350,6 +348,15 @@ def testAtan2(self):
self.assertTrue(math.isnan(math.atan2(NAN, 2.3)))
self.assertTrue(math.isnan(math.atan2(NAN, INF)))
self.assertTrue(math.isnan(math.atan2(NAN, NAN)))
# overflow and underflow of y/x
self.assertEqual(math.atan2(1E-320, 1E300), 0.0)
self.assertEqual(math.atan2(-1E-320, 1E300), -0.0)
self.assertAlmostEqual(math.atan2(1E300, 1E-320), math.pi/2)
self.assertAlmostEqual(math.atan2(1E300, -1E-320), math.pi/2)
self.assertAlmostEqual(math.atan2(1E-320, -1E300), math.pi)
self.assertAlmostEqual(math.atan2(-1E-320, -1E300), -math.pi)
self.assertAlmostEqual(math.atan2(-1E300, 1E-320), -math.pi/2)
self.assertAlmostEqual(math.atan2(-1E300, -1E-320), -math.pi/2)

def testCbrt(self):
self.assertRaises(TypeError, math.cbrt)
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Library/2026-07-20-09-35-32.gh-issue-153144.0a2AoO.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
:func:`math.atan2` and :func:`math.atan2pi` no longer raise :exc:`ValueError`.
:func:`cmath.phase` no longer raises :exc:`OverflowError` or :exc:`ValueError`,
and the values of ``math.atan2(b, a)`` and ``cmath.phase(complex(a, b))`` always
match. Contributed by High Performance Kernels LLC.
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2026-10-02-22-32-10.gh-issue-156145.9l_tZb.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The function :func:`math.copysign` no longer sets ``errno`` to zero.
Contributed by High Performance Kernels LLC.
Comment thread
hpkfft marked this conversation as resolved.
11 changes: 5 additions & 6 deletions Modules/cmathmodule.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1004,12 +1004,11 @@ cmath_phase_impl(PyObject *module, Py_complex z)
{
double phi;

errno = 0;
phi = atan2(z.imag, z.real); /* should not cause any exception */
if (errno != 0)
return math_error();
Comment thread
skirpichev marked this conversation as resolved.
else
return PyFloat_FromDouble(phi);
phi = atan2(z.imag, z.real);
/* gh-153144: Ignore atan2() errno on purpose since it can optionally be
Comment thread
picnixz marked this conversation as resolved.
EDOM, which we should ignore, or ERANGE if phi underflows,
which is silent on Python. Overflow is not possible. */
return PyFloat_FromDouble(phi);
}

/*[clinic input]
Expand Down
43 changes: 38 additions & 5 deletions Modules/mathmodule.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ math_1a(PyObject *arg, double (*func) (double), const char *err_msg)
The last rule is used to catch overflow on platforms which follow
C89 but for which HUGE_VAL is not an infinity.

For most two-argument functions (copysign, fmod, hypot, atan2)
For most two-argument functions (fmod, hypot)

skirpichev Oct 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Actually, the only remaining math_2()-type function is remainder()...

hpkfft Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think (fmod, hypot) in the comment above refers to the C math library functions.
Unless I've missed it, Python does not use the C math library function double remainder(double, double), so I don't think we should add it to this comment.

skirpichev Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think (fmod, hypot) in the comment above refers to the C math library functions.

I don't think so. Before 53876d9, both fmod and hypot were using FUNC2 macro.

Python does not use the C math library function double remainder(double, double)

Yes, but m_remainder (name in style m_log and co, suggesting a workarounded libm's function) is just an implementation of Annex F remainder.

Perhaps, nowadays we could replace this function by standard library call: at least this works well for me locally. See previous attempt of removing legacy stuff: #101679

Edit: FYI, #158826.

hpkfft Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Interesting. But it seems sufficiently unrelated as to allow this PR to be merged without further tweaks to the comments.
these rules are enough to ensure that Python's functions behave as
specified in 'Annex F' of the C99 standard, with the 'invalid' and
'divide-by-zero' floating-point exceptions mapping to Python's
Expand All @@ -1030,8 +1030,9 @@ math_2(PyObject *const *args, Py_ssize_t nargs,
double (*func) (double, double), const char *funcname)
{
double x, y, r;
if (!_PyArg_CheckPositional(funcname, nargs, 2, 2))
if (!_PyArg_CheckPositional(funcname, nargs, 2, 2)) {
Comment thread
skirpichev marked this conversation as resolved.
return NULL;
}
x = PyFloat_AsDouble(args[0]);
if (x == -1.0 && PyErr_Occurred()) {
return NULL;
Expand Down Expand Up @@ -1060,6 +1061,29 @@ math_2(PyObject *const *args, Py_ssize_t nargs,
return PyFloat_FromDouble(r);
}

/* variant of math_2, to be used when the function being wrapped is known NOT
to need error checking (i.e., no overflow, invalid, or divide-by-zero). */
Comment on lines +1064 to +1065

skirpichev Oct 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I suggest expand this with atan2() example and remove other comments below.

static PyObject *
math_2ne(PyObject *const *args, Py_ssize_t nargs,
double (*func) (double, double), const char *funcname)
{
double x, y, r;
if (!_PyArg_CheckPositional(funcname, nargs, 2, 2)) {
return NULL;
Comment thread
picnixz marked this conversation as resolved.
}
x = PyFloat_AsDouble(args[0]);
if (x == -1.0 && PyErr_Occurred()) {
return NULL;
}
y = PyFloat_AsDouble(args[1]);
if (y == -1.0 && PyErr_Occurred()) {
return NULL;
}
r = (*func)(x, y); // Ignore errno on purpose.
Comment thread
hpkfft marked this conversation as resolved.

skirpichev Oct 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
r = (*func)(x, y); // Ignore errno on purpose.
r = (*func)(x, y);

hpkfft Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@vstinner asked for this comment on August 14: #153148 (comment)
return PyFloat_FromDouble(r);
}

#define FUNC1(funcname, func, can_overflow, docstring) \
static PyObject * math_##funcname(PyObject *self, PyObject *args) { \
return math_1(args, func, can_overflow, NULL); \
Expand Down Expand Up @@ -1090,6 +1114,12 @@ math_2(PyObject *const *args, Py_ssize_t nargs,
}\
PyDoc_STRVAR(math_##funcname##_doc, docstring);

#define FUNC2NE(funcname, func, docstring) \
static PyObject * math_##funcname(PyObject *self, PyObject *const *args, Py_ssize_t nargs) { \
return math_2ne(args, nargs, func, #funcname); \
}\
PyDoc_STRVAR(math_##funcname##_doc, docstring);

FUNC1D(acos, acos, 0,
"acos($module, x, /)\n--\n\n"
"Return the arc cosine (measured in radians) of x.\n\n"
Expand Down Expand Up @@ -1121,11 +1151,14 @@ FUNC1(atan, atan, 0,
"atan($module, x, /)\n--\n\n"
"Return the arc tangent (measured in radians) of x.\n\n"
"The result is between -pi/2 and pi/2.")
FUNC2(atan2, atan2,
/* gh-153144: Ignore atan2 and atan2pi errno on purpose since it can optionally
Comment thread
hpkfft marked this conversation as resolved.
* be EDOM, which we should ignore, or ERANGE on underflow, which is
* silent on Python. Overflow is not possible. */
Comment on lines +1154 to +1156

skirpichev Oct 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
/* gh-153144: Ignore atan2 and atan2pi errno on purpose since it can optionally
* be EDOM, which we should ignore, or ERANGE on underflow, which is
* silent on Python. Overflow is not possible. */

hpkfft Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@vstinner asked for the "ignore errno on purpose" part of this comment on August 14: #153148 (comment)
@picnixz asked to "document the reason why we ignore errno in this case" on September 18: #153148 (comment)
FUNC2NE(atan2, atan2,
"atan2($module, y, x, /)\n--\n\n"
"Return the arc tangent (measured in radians) of y/x.\n\n"
"Unlike atan(y/x), the signs of both x and y are considered.")
FUNC2(atan2pi, m_atan2pi,
FUNC2NE(atan2pi, m_atan2pi,
"atan2pi($module, y, x, /)\n--\n\n"
"Return the arc tangent (measured in half-turns) of y/x.\n\n"
"Unlike atanpi(y/x), the signs of both x and y are considered.")
Expand Down Expand Up @@ -1178,7 +1211,7 @@ math_ceil(PyObject *module, PyObject *number)
return PyLong_FromDouble(ceil(x));
}

FUNC2(copysign, copysign,
FUNC2NE(copysign, copysign,
Comment thread
picnixz marked this conversation as resolved.
"copysign($module, x, y, /)\n--\n\n"
"Return a float with the magnitude (absolute value) of x but the sign of y.\n\n"
"On platforms that support signed zeros, copysign(1.0, -0.0)\n"
Expand Down
Loading
Back | FazBrowse Home | New Git URL