-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-101410: Improve error messages in math module #101492
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
Changes from all commits
85edd15
2448fe4
c0c1e2d
1a491f2
4cd75b0
611dedb
ef68c24
dc8e593
75a7c5c
4b2ec23
3fc73e9
30be617
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Make several math functions support custom error messages. :func:`math.sqrt` and :func:`math.log` | ||
now provide more helpful error messages. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -921,7 +921,7 @@ is_error(double x) | |||||
*/ | ||||||
|
||||||
static PyObject * | ||||||
math_1(PyObject *arg, double (*func) (double), int can_overflow) | ||||||
math_1(PyObject *arg, double (*func) (double), int can_overflow, const char *err_msg) | ||||||
{ | ||||||
double x, r; | ||||||
x = PyFloat_AsDouble(arg); | ||||||
|
@@ -930,8 +930,7 @@ math_1(PyObject *arg, double (*func) (double), int can_overflow) | |||||
errno = 0; | ||||||
r = (*func)(x); | ||||||
if (isnan(r) && !isnan(x)) { | ||||||
PyErr_SetString(PyExc_ValueError, | ||||||
"math domain error"); /* invalid arg */ | ||||||
PyErr_Format(PyExc_ValueError, err_msg, arg); /* invalid arg */ | ||||||
return NULL; | ||||||
} | ||||||
if (isinf(r) && isfinite(x)) { | ||||||
|
@@ -1030,9 +1029,9 @@ math_2(PyObject *const *args, Py_ssize_t nargs, | |||||
return PyFloat_FromDouble(r); | ||||||
} | ||||||
|
||||||
#define FUNC1(funcname, func, can_overflow, docstring) \ | ||||||
#define FUNC1(funcname, func, can_overflow, docstring, err_msg) \ | ||||||
static PyObject * math_##funcname(PyObject *self, PyObject *args) { \ | ||||||
return math_1(args, func, can_overflow); \ | ||||||
return math_1(args, func, can_overflow, err_msg); \ | ||||||
}\ | ||||||
PyDoc_STRVAR(math_##funcname##_doc, docstring); | ||||||
|
||||||
|
@@ -1051,31 +1050,38 @@ math_2(PyObject *const *args, Py_ssize_t nargs, | |||||
FUNC1(acos, acos, 0, | ||||||
"acos($module, x, /)\n--\n\n" | ||||||
"Return the arc cosine (measured in radians) of x.\n\n" | ||||||
"The result is between 0 and pi.") | ||||||
"The result is between 0 and pi.", | ||||||
"math domain error") | ||||||
FUNC1(acosh, acosh, 0, | ||||||
"acosh($module, x, /)\n--\n\n" | ||||||
"Return the inverse hyperbolic cosine of x.") | ||||||
"Return the inverse hyperbolic cosine of x.", | ||||||
"math domain error") | ||||||
FUNC1(asin, asin, 0, | ||||||
"asin($module, x, /)\n--\n\n" | ||||||
"Return the arc sine (measured in radians) of x.\n\n" | ||||||
"The result is between -pi/2 and pi/2.") | ||||||
"The result is between -pi/2 and pi/2.", | ||||||
"math domain error") | ||||||
FUNC1(asinh, asinh, 0, | ||||||
"asinh($module, x, /)\n--\n\n" | ||||||
"Return the inverse hyperbolic sine of x.") | ||||||
"Return the inverse hyperbolic sine of x.", | ||||||
"math domain error") | ||||||
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.") | ||||||
"The result is between -pi/2 and pi/2.", | ||||||
"math domain error") | ||||||
FUNC2(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.") | ||||||
FUNC1(atanh, atanh, 0, | ||||||
"atanh($module, x, /)\n--\n\n" | ||||||
"Return the inverse hyperbolic tangent of x.") | ||||||
"Return the inverse hyperbolic tangent of x.", | ||||||
"math domain error") | ||||||
FUNC1(cbrt, cbrt, 0, | ||||||
"cbrt($module, x, /)\n--\n\n" | ||||||
"Return the cube root of x.") | ||||||
"Return the cube root of x.", | ||||||
"math domain error") | ||||||
|
||||||
/*[clinic input] | ||||||
math.ceil | ||||||
|
@@ -1121,10 +1127,12 @@ FUNC2(copysign, copysign, | |||||
"returns -1.0.\n") | ||||||
FUNC1(cos, cos, 0, | ||||||
"cos($module, x, /)\n--\n\n" | ||||||
"Return the cosine of x (measured in radians).") | ||||||
"Return the cosine of x (measured in radians).", | ||||||
"math domain error") | ||||||
FUNC1(cosh, cosh, 1, | ||||||
"cosh($module, x, /)\n--\n\n" | ||||||
"Return the hyperbolic cosine of x.") | ||||||
"Return the hyperbolic cosine of x.", | ||||||
"math domain error") | ||||||
FUNC1A(erf, erf, | ||||||
"erf($module, x, /)\n--\n\n" | ||||||
"Error function at x.") | ||||||
|
@@ -1133,18 +1141,22 @@ FUNC1A(erfc, erfc, | |||||
"Complementary error function at x.") | ||||||
FUNC1(exp, exp, 1, | ||||||
"exp($module, x, /)\n--\n\n" | ||||||
"Return e raised to the power of x.") | ||||||
"Return e raised to the power of x.", | ||||||
"math domain error") | ||||||
FUNC1(exp2, exp2, 1, | ||||||
"exp2($module, x, /)\n--\n\n" | ||||||
"Return 2 raised to the power of x.") | ||||||
"Return 2 raised to the power of x.", | ||||||
"math domain error") | ||||||
FUNC1(expm1, expm1, 1, | ||||||
"expm1($module, x, /)\n--\n\n" | ||||||
"Return exp(x)-1.\n\n" | ||||||
"This function avoids the loss of precision involved in the direct " | ||||||
"evaluation of exp(x)-1 for small x.") | ||||||
"evaluation of exp(x)-1 for small x.", | ||||||
"math domain error") | ||||||
FUNC1(fabs, fabs, 0, | ||||||
"fabs($module, x, /)\n--\n\n" | ||||||
"Return the absolute value of the float x.") | ||||||
"Return the absolute value of the float x.", | ||||||
"math domain error") | ||||||
|
||||||
/*[clinic input] | ||||||
math.floor | ||||||
|
@@ -1192,7 +1204,8 @@ FUNC1A(lgamma, m_lgamma, | |||||
FUNC1(log1p, m_log1p, 0, | ||||||
"log1p($module, x, /)\n--\n\n" | ||||||
"Return the natural logarithm of 1+x (base e).\n\n" | ||||||
"The result is computed in a way which is accurate for x near zero.") | ||||||
"The result is computed in a way which is accurate for x near zero.", | ||||||
"math domain error") | ||||||
FUNC2(remainder, m_remainder, | ||||||
"remainder($module, x, y, /)\n--\n\n" | ||||||
"Difference between x and the closest integer multiple of y.\n\n" | ||||||
|
@@ -1201,19 +1214,25 @@ FUNC2(remainder, m_remainder, | |||||
"y, the nearest even value of n is used. The result is always exact.") | ||||||
FUNC1(sin, sin, 0, | ||||||
"sin($module, x, /)\n--\n\n" | ||||||
"Return the sine of x (measured in radians).") | ||||||
"Return the sine of x (measured in radians).", | ||||||
"math domain error") | ||||||
FUNC1(sinh, sinh, 1, | ||||||
"sinh($module, x, /)\n--\n\n" | ||||||
"Return the hyperbolic sine of x.") | ||||||
"Return the hyperbolic sine of x.", | ||||||
"math domain error") | ||||||
FUNC1(sqrt, sqrt, 0, | ||||||
"sqrt($module, x, /)\n--\n\n" | ||||||
"Return the square root of x.") | ||||||
"Return the square root of x.", | ||||||
"math.sqrt() expects a nonnegative input, got '%R'.\n" | ||||||
"See cmath.sqrt() for a variant that supports complex numbers") | ||||||
FUNC1(tan, tan, 0, | ||||||
"tan($module, x, /)\n--\n\n" | ||||||
"Return the tangent of x (measured in radians).") | ||||||
"Return the tangent of x (measured in radians).", | ||||||
"math domain error") | ||||||
FUNC1(tanh, tanh, 0, | ||||||
"tanh($module, x, /)\n--\n\n" | ||||||
"Return the hyperbolic tangent of x.") | ||||||
"Return the hyperbolic tangent of x.", | ||||||
"math domain error") | ||||||
|
||||||
/* Precision summation function as msum() by Raymond Hettinger in | ||||||
<https://code.activestate.com/recipes/393090-binary-floating-point-summation-accurate-to-full-p/>, | ||||||
|
@@ -2180,7 +2199,7 @@ math_modf_impl(PyObject *module, double x) | |||||
in that int is larger than PY_SSIZE_T_MAX. */ | ||||||
|
||||||
static PyObject* | ||||||
loghelper(PyObject* arg, double (*func)(double)) | ||||||
loghelper(PyObject* arg, double (*func)(double), const char *err_msg) | ||||||
{ | ||||||
/* If it is int, do it ourselves. */ | ||||||
if (PyLong_Check(arg)) { | ||||||
|
@@ -2189,8 +2208,7 @@ loghelper(PyObject* arg, double (*func)(double)) | |||||
|
||||||
/* Negative or zero inputs give a ValueError. */ | ||||||
if (!_PyLong_IsPositive((PyLongObject *)arg)) { | ||||||
PyErr_SetString(PyExc_ValueError, | ||||||
"math domain error"); | ||||||
PyErr_Format(PyExc_ValueError, err_msg, arg); | ||||||
return NULL; | ||||||
} | ||||||
|
||||||
|
@@ -2214,7 +2232,7 @@ loghelper(PyObject* arg, double (*func)(double)) | |||||
} | ||||||
|
||||||
/* Else let libm handle it by itself. */ | ||||||
return math_1(arg, func, 0); | ||||||
return math_1(arg, func, 0, err_msg); | ||||||
} | ||||||
|
||||||
|
||||||
|
@@ -2229,11 +2247,11 @@ math_log(PyObject *module, PyObject * const *args, Py_ssize_t nargs) | |||||
if (!_PyArg_CheckPositional("log", nargs, 1, 2)) | ||||||
return NULL; | ||||||
|
||||||
num = loghelper(args[0], m_log); | ||||||
num = loghelper(args[0], m_log, "math.log() expects a positive input, got '%R'."); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think using quotes around numbers is relevant. For instance, if it's a custom class of a float that overrides
Suggested change
|
||||||
if (num == NULL || nargs == 1) | ||||||
return num; | ||||||
|
||||||
den = loghelper(args[1], m_log); | ||||||
den = loghelper(args[1], m_log, "math.log() expects a positive input, got '%R'."); | ||||||
if (den == NULL) { | ||||||
Py_DECREF(num); | ||||||
return NULL; | ||||||
|
@@ -2263,7 +2281,7 @@ static PyObject * | |||||
math_log2(PyObject *module, PyObject *x) | ||||||
/*[clinic end generated code: output=5425899a4d5d6acb input=08321262bae4f39b]*/ | ||||||
{ | ||||||
return loghelper(x, m_log2); | ||||||
return loghelper(x, m_log2, "math.log2() expects a positive input, got '%R'."); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. |
||||||
} | ||||||
|
||||||
|
||||||
|
@@ -2280,7 +2298,7 @@ static PyObject * | |||||
math_log10(PyObject *module, PyObject *x) | ||||||
/*[clinic end generated code: output=be72a64617df9c6f input=b2469d02c6469e53]*/ | ||||||
{ | ||||||
return loghelper(x, m_log10); | ||||||
return loghelper(x, m_log10, "math.log10() expects a positive input, got '%R'."); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto |
||||||
} | ||||||
|
||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't same message be used below on the line 1076 as well (infinite r for finite input)? atanh(-1) is an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if there is a function that can have both input and output( NAN r for non-NAN x and infinite r for finite x).
If such a function exists, it may lead to confusing error messages, because one error message corresponds to two scenarios. I haven't thought of such a function yet, please suggest if it exists. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same atanh :)
atanh(2) - for the 1st scenario and atanh(1) - for the second. I doubt it will be confusing if you raise instead something like ValueError("atanh expects an argument from (-1, -1), got %R"). I don't think it does make sense to have a different descriptions for pole errors (like atanh(1)). @mdickinson ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 9c32ae0 from #102523.