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.

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

gh-101410: customize error messages for 1-arg math functions #129497

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+41 −32
Open

gh-101410: customize error messages for 1-arg math functions #129497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
77a643c
gh-101410: customize error messages for 1-arg math functions
skirpichev Jan 31, 2025
d88b55f
+ news
skirpichev Jan 31, 2025
b99661e
Update Lib/test/test_math.py
skirpichev Feb 4, 2025
d57a640
Merge branch 'master' into math-err-msg/101410
skirpichev Feb 6, 2025
81ae026
address review: correct gamma/lgamma error msgs
skirpichev Feb 6, 2025
11365cb
Update Misc/NEWS.d/next/Library/2023-02-01-16-41-31.gh-issue-101410.D…
skirpichev Feb 6, 2025
53b68c2
address review: fix typo in news & gamma/lgamma error messages
skirpichev Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions Lib/test/mathdata/ieee754.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,31 +127,31 @@ Trigonometric Functions
>>> sin(INF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a finite input, got inf
>>> sin(NINF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a finite input, got -inf
>>> sin(NAN)
nan
>>> cos(INF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a finite input, got inf
>>> cos(NINF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a finite input, got -inf
>>> cos(NAN)
nan
>>> tan(INF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a finite input, got inf
>>> tan(NINF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a finite input, got -inf
>>> tan(NAN)
nan

Expand All @@ -169,11 +169,11 @@ True
>>> asin(INF), asin(NINF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a number in range from -1 up to 1, got inf
>>> acos(INF), acos(NINF)
Traceback (most recent call last):
...
ValueError: math domain error
ValueError: expected a number in range from -1 up to 1, got inf
>>> equal(atan(INF), PI/2), equal(atan(NINF), -PI/2)
(True, True)

Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_math.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2533,10 +2533,10 @@ def test_exception_messages(self):
math.log(x)
x = -123
with self.assertRaisesRegex(ValueError,
f"expected a positive input, got {x}"):
"expected a positive input"):
math.log(x)
with self.assertRaisesRegex(ValueError,
f"expected a float or nonnegative integer, got {x}"):
f"expected a noninteger or positive integer, got {x}"):
math.gamma(x)
x = 1.0
with self.assertRaisesRegex(ValueError,
Expand Down
5 changes: 2 additions & 3 deletions Misc/NEWS.d/next/Library/2023-02-01-16-41-31.gh-issue-101410.Dt2aQE.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
Support custom messages for domain errors in the :mod:`math` module
(:func:`math.sqrt`, :func:`math.log` and :func:`math.atanh` were modified as
examples). Patch by Charlie Zhao and Sergey B Kirpichev.
Added more detailed messages for domain errors in the :mod:`math` module.
Patch by Charlie Zhao and Sergey B Kirpichev.
48 changes: 29 additions & 19 deletions Modules/mathmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1089,17 +1089,20 @@ math_2(PyObject *const *args, Py_ssize_t nargs,
}\
PyDoc_STRVAR(math_##funcname##_doc, docstring);

FUNC1(acos, acos, 0,
FUNC1D(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.")
FUNC1(acosh, acosh, 0,
"The result is between 0 and pi.",
"expected a number in range from -1 up to 1, got %s")
Copy link
Member

Choose a reason for hiding this comment

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

We could say "a number in the interval [-1, 1]" though I'm a bit worried about the notation that may not be understood (for non mathematicians). Otherwise shouldn't we say "in the range from -1 up to 1" ? (I'm not sure here for the English part so @AA-Turner could help us)

FUNC1D(acosh, acosh, 0,
"acosh($module, x, /)\n--\n\n"
"Return the inverse hyperbolic cosine of x.")
FUNC1(asin, asin, 0,
"Return the inverse hyperbolic cosine of x.",
"expected argument value not less than 1, got %s")
FUNC1D(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.",
"expected a number in range from -1 up to 1, got %s")
FUNC1(asinh, asinh, 0,
"asinh($module, x, /)\n--\n\n"
"Return the inverse hyperbolic sine of x.")
Expand Down Expand Up @@ -1161,9 +1164,10 @@ FUNC2(copysign, copysign,
"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"
"returns -1.0.\n")
FUNC1(cos, cos, 0,
FUNC1D(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).",
"expected a finite input, got %s")
Copy link
Member

Choose a reason for hiding this comment

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

It works for NaN which is not a finite number.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we should clutter the message with nans stuff. It's expected, that math functions accept nans: garbage in - garbage out.

Copy link
Member

Choose a reason for hiding this comment

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

The point of changing the error messages is to make them more informative, not misleading. If we cannot improve the error message, it is better to leave the old unspecific message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, then in this manner we should change all error messages. All will require a specific case for nan. I.e. instead of "expected a number in range from -1 up to 1, got inf" we should get something like "expected a number in range from -1 up to 1, or nan, got inf".

I don't think it's a helpful information for readers. We shouldn't clutter the function domain description with details on nan propagation.

Copy link
Member

Choose a reason for hiding this comment

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

"Finite" is not a colloquial term. It can be understood only in context of floating-point numbers, and there it means not infinity and not NaN.

Yes, other messages may need a correction too. Instead of "expected a number in range from -1 up to 1, got inf" it can say "inf is outside of range from -1 up to 1" or something like.

The error messages should be correct and unambiguous, otherwise there is no point in changing the old error messages.

Copy link
Member Author

Choose a reason for hiding this comment

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

"Finite" is not a colloquial term. It can be understood only in context of floating-point numbers

But we are in this context.

Instead of "expected a number in range from -1 up to 1, got inf" it can say "inf is outside of range from -1 up to 1" or something like. [...] The error messages should be correct and unambiguous

Hmm, I'm not sure that 2nd variant much better for nans. Reading this error message I also can't say whether nan is an acceptable input or not.

Also, in such versions you will rather describe the complement of the function domain. E.g. instead of "expected a nonnegative input, got -1.0" - something like "-1.0 is not a nonnegative number"

Copy link
Member

Choose a reason for hiding this comment

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

But we are in this context.

And in that context math.isfinite() returns false for NaN.

Also, in such versions you will rather describe the complement of the function domain. E.g. instead of "expected a nonnegative input, got -1.0" - something like "-1.0 is not a nonnegative number"

Rather "-1.0 is a negative integer number". If expression for the complement of the function domain is simpler, it should be used instead. In any case, all these messages should be reviewed by native speakers with mathematical or computer science background and teaching experience. cc @rhettinger, @tim-one, @mdickinson

Copy link
Member Author

Choose a reason for hiding this comment

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

Rather "-1.0 is a negative integer number".

With this wording it says something about the input, rather than about the function domain.

all these messages should be reviewed by native speakers with mathematical or computer science background and teaching experience.

I can only appreciate that.

As summary, in the proposed patch we have error messages like "expected number in , got %s", which is along the line, suggested by Raymond in the issue description. Problem (well, maybe): this doesn't account the nan as an acceptable input.

FUNC1(cosh, cosh, 1,
"cosh($module, x, /)\n--\n\n"
"Return the hyperbolic cosine of x.")
Expand Down Expand Up @@ -1228,33 +1232,37 @@ math_floor(PyObject *module, PyObject *number)
FUNC1AD(gamma, m_tgamma,
"gamma($module, x, /)\n--\n\n"
"Gamma function at x.",
"expected a float or nonnegative integer, got %s")
FUNC1A(lgamma, m_lgamma,
"expected a noninteger or positive integer, got %s")
FUNC1AD(lgamma, m_lgamma,
"lgamma($module, x, /)\n--\n\n"
"Natural logarithm of absolute value of Gamma function at x.")
FUNC1(log1p, m_log1p, 0,
"Natural logarithm of absolute value of Gamma function at x.",
"expected a noninteger or positive integer, got %s")
FUNC1D(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.",
"expected argument value > -1, got %s")
FUNC2(remainder, m_remainder,
"remainder($module, x, y, /)\n--\n\n"
"Difference between x and the closest integer multiple of y.\n\n"
"Return x - n*y where n*y is the closest integer multiple of y.\n"
"In the case where x is exactly halfway between two multiples of\n"
"y, the nearest even value of n is used. The result is always exact.")
FUNC1(sin, sin, 0,
FUNC1D(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).",
"expected a finite input, got %s")
FUNC1(sinh, sinh, 1,
"sinh($module, x, /)\n--\n\n"
"Return the hyperbolic sine of x.")
FUNC1D(sqrt, sqrt, 0,
"sqrt($module, x, /)\n--\n\n"
"Return the square root of x.",
"expected a nonnegative input, got %s")
FUNC1(tan, tan, 0,
FUNC1D(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).",
"expected a finite input, got %s")
FUNC1(tanh, tanh, 0,
"tanh($module, x, /)\n--\n\n"
"Return the hyperbolic tangent of x.")
Expand Down Expand Up @@ -2232,8 +2240,10 @@ loghelper(PyObject* arg, double (*func)(double))

/* Negative or zero inputs give a ValueError. */
if (!_PyLong_IsPositive((PyLongObject *)arg)) {
PyErr_Format(PyExc_ValueError,
"expected a positive input, got %S", arg);
/* The input can be an arbitrary large integer, so we
don't include it's value in the error message. */
PyErr_SetString(PyExc_ValueError,
"expected a positive input");
return NULL;
}

Expand Down
Loading