gh-101410: support custom messages for domain errors in the math module

[skirpichev]

This adds basic support to override default messages for domain errors in the math_1() and math_1a() helpers. The sqrt(), atanh(), log2(), log10(), log() and gamma() functions were modified as examples. New macros support gradual changing of error messages in other 1-arg functions.

• Issue: Customize and improve error messages in the math module. #101410

[skirpichev]

#101492 somehow stuck, here I would like to continue this work. @CharlieZhao95, are you ok with this?

The given pr introduces a new macro and change only few functions, that covers all cases in math_1(). I hope, this also address reviews in the mentioned pr (tests added, error messages print converted argument, etc).

I kept only short error messages. Not sure if it worth printing the function name, as it usually shown in tracebacks.

>>> import math
>>> for i in range(5, -10, -2):
...     print(math.log(i))
...     
1.6094379124341003
1.0986122886681098
0.0
Traceback (most recent call last):
  File "<python-input-1>", line 2, in <module>
    print(math.log(i))
          ~~~~~~~~^^^
ValueError: expected a positive input, got -1

Not sure also if we must suggest cmath in error messages. But if we decide to do so, probably it does make sense for almost every domain error in the math module and we can do this with a template.

diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c
index 665b02e702..c4516fabda 100644
--- a/Modules/mathmodule.c
+++ b/Modules/mathmodule.c
@@ -1046,7 +1046,11 @@ math_2(PyObject *const *args, Py_ssize_t nargs,
 
 #define FUNC1D(funcname, func, can_overflow, docstring, err_msg)        \
     static PyObject * math_##funcname(PyObject *self, PyObject *args) { \
-        return math_1(args, func, can_overflow, err_msg);               \
+        return math_1(args, func, can_overflow,                         \
+                      (err_msg                                          \
+                       "\nSee cmath."                                   \
+                       #funcname                                        \
+                       "(), that supports complex numbers"));           \
     }\
     PyDoc_STRVAR(math_##funcname##_doc, docstring);
 

CC @rhettinger as author of the issue, I would appreciate feedback
CC @serhiy-storchaka

[picnixz]

Looks good to me. Just some nitpicks.

[serhiy-storchaka]

More informative error messages is good, but I afraid that the cost may be too high. If this is really an error due to invalid input data, then some cost is more tolerable. But if the exception is used in the EAFP style of solving computational error (for example, the argument of sqrt() in sqrt(b*b-4*a*c) can be a small negative number due to limited precision computation and can be replaced by 0), additional cost harms the average performance.

[skirpichev]

additional cost harms the average performance.

But it's only printing. Currently, math_1() uses PyFloat_FromDouble() for converted by PyFloat_AsDouble() value; well, maybe I should just use PyOS_double_to_string(). Anyway it doesn't seems to be too expensive in either way.

On another hand, with this - we don't print the original value of the argument (something like Fraction(1/2**1000). Maybe it's better to omit the value at all?

[serhiy-storchaka]

If include the value, it should be a converted value, not the original value, because conversion can change the value (for example turn a small positive value into 0). The repr of the original value has also larger chance to produce long and expensive result.

[skirpichev]

If include the value, it should be a converted value, not the original value

This is how it works. loghelper() is an exception.

I see only one solution for loghelper(): use two messages. One for PyLong-branch (this message will come from loghelper argument to distinguish arguments of 2-arg form) and a common template for the math_1() fallback.

[serhiy-storchaka]

Thank you. You addressed all my concerns. Extreme slow cases were removed, and formatting error messages in common case was optimized.

I would approve this PR, but I want first to know opinion of the author of the request, @rhettinger.

And I think that we can add custom error messages for more functions: acos(), asin(), gamma(), lgamma(). Maybe pow(), where rules are complex.

[skirpichev]

And I think that we can add custom error messages for more functions: acos(), asin(), gamma(), lgamma(). Maybe pow(), where rules are complex.

I would prefer to make first required changes in helpers. This pr changes math_1(). Following pr can add custom error messages for all math_1-based functions, it will be mostly mechanical process.

Then we can address e.g. atan2 and pow.

[CharlieZhao95]

Thank you for continuing this work! A simple comment here :)

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[skirpichev]

I have made the requested changes; please review again

[bedevere-app]

Thanks for making the requested changes!

@JelleZijlstra: please review the changes made to this pull request.

[skirpichev]

Ok, as Serhiy interested in Raymond's opinion, I've asked for review.

Meanwhile, I've added one another commit (I feel this pr isn't too big for review). With this, all 1-arg functions should be covered. I would prefer, if specific error messages will be added in a separate pr. PR description adjusted.