Skip to content
12 changes: 10 additions & 2 deletions Lib/test/test_cmath.py
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
2 changes: 0 additions & 2 deletions Lib/test/test_math.py
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Given a complex number ``z`` for which ``z.imag/z.real`` underflows to zero,
the :func:`cmath.phase` of ``z`` should not raise ``OverflowError``. Also,
given arguments of zero, Python's :func:`math.atan2`, :func:`math.atan2pi`,
and :func:`cmath.phase` should return the values specified by Annex F of the
C standard, not raise ``ValueError``, regardless of whether the platform's
math library sets ``errno`` for ``atan2(0.0, 0.0)``, ``atan2(0.0, -0.0)``, etc.
Contributed by High Performance Kernels LLC.
7 changes: 2 additions & 5 deletions Modules/cmathmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1004,12 +1004,9 @@ cmath_phase_impl(PyObject *module, Py_complex z)
{
double phi;

errno = 0;
phi = atan2(z.imag, z.real); /* should not cause any exception */
Comment thread
hpkfft marked this conversation as resolved.
if (errno != 0)
return math_error();

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.

I expected a test_cmath failure when this code path is removed. Is it because glibc math library doesn't errno in this case?

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.

Yes. With the exception of the Intel and Solaris math libraries, errno is not set in this case by any math library that Python cares about. I say this because test_phase in Lib/test/test_cmath.py has asserted that return values are correct since Python 3.14, and nobody has complained. If errno were set by the C math library, the unittest would fail with ValueError: math domain error.

🌱 Some math libraries (e.g., musl) don't set errno for anything, so Python cannot rely on errno for detecting overflow or invalid. I would think that errno checking can be removed everywhere....

else
return PyFloat_FromDouble(phi);
/* gh-153144: Ignore atan2() errno on purpose. */
Comment thread
hpkfft marked this conversation as resolved.
return PyFloat_FromDouble(phi);
}

/*[clinic input]
Expand Down
36 changes: 32 additions & 4 deletions Modules/mathmodule.c
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)
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 Down Expand Up @@ -1060,6 +1060,28 @@ 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). */

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;
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.
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 +1112,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 +1149,11 @@ 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,
FUNC2NE(atan2, atan2, // gh-153144: Ignore atan2() errno on purpose.
Comment thread
hpkfft marked this conversation as resolved.
"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, // gh-153144: Ignore atan2pi() errno on purpose.
Comment thread
hpkfft marked this conversation as resolved.
"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 +1206,7 @@ math_ceil(PyObject *module, PyObject *number)
return PyLong_FromDouble(ceil(x));
}

FUNC2(copysign, copysign,
FUNC2NE(copysign, copysign,
"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