gh-101410: Improve error messages in math module #101492

CharlieZhao95 · 2023-02-01T09:09:19Z

Make several math functions support custom error messages.
math.sqrt() and math.log() now provide more helpful error messages.

>>> import math
# before
>>> math.sqrt(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math domain error

# now
>>> math.sqrt(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math.sqrt() expects a non-negative input, got '-1'.
See cmath.sqrt() for variation that supports complex numbers

>>> math.log(0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math.log() expects a positive input, got '0'.

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

OTheDev · 2023-02-02T05:23:53Z

Modules/mathmodule.c

-      "Return the square root of x.")
+      "Return the square root of x.",
+      "math.sqrt() expects a non-negative input, got '%R'.\n"
+      "See cmath.sqrt() for variation that supports complex numbers")


Perhaps an a before variation would be more correct? Or

See cmath.sqrt(), which also supports negative inputs. I don't know what's better - just a suggestion (:

Perhaps an a before variation would be more correct? Or

See cmath.sqrt(), which also supports negative inputs. I don't know what's better - just a suggestion (:

Thanks! It seems reasonable to me to add a. Let's wait for others to reply. :)

CharlieZhao95 · 2023-02-03T02:18:45Z

I guess similar functions and macros can have the same changes (eg FUNC1A, FUNC2). I will continue my work after the initial review. :)

Modules/mathmodule.c

…h_error_message

skirpichev · 2023-02-18T12:57:40Z

Modules/mathmodule.c

@@ -1063,8 +1064,7 @@ math_1_to_whatever(PyObject *arg, double (*func) (double),
    errno = 0;
    r = (*func)(x);
    if (Py_IS_NAN(r) && !Py_IS_NAN(x)) {
-        PyErr_SetString(PyExc_ValueError,
-                        "math domain error"); /* invalid arg */
+        PyErr_Format(PyExc_ValueError, err_msg, arg); /* invalid arg */


Shouldn't same message be used below on the line 1076 as well (infinite r for finite input)? atanh(-1) is an example.

Shouldn't same message be used below on the line 1076 as well (infinite r for finite input)? atanh(-1) is an example.

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. :)

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 ?

See 9c32ae0 from #102523.

serhiy-storchaka

Would not be better to output the converted C double value x instead of the original Python object arg? The repr of the Python object can be arbitrary long, unlike to the string representation of C double. Also, there may be some errors introduced at converting Python object to C double, not visible from its repr. Although it is much more cumbersome.

serhiy-storchaka · 2023-09-23T16:20:41Z

For example, math.log10(Fraction(1, 10**400)) raises an exception, even if the argument is positive. On other hand math.sqrt(Fraction(-1, 10**400)) returns -0.0 even if the argument is negative. BTW, math.sqrt() returning negative zero can cause issues in formatted output.

CharlieZhao95 · 2023-09-26T08:33:39Z

Would not be better to output the converted C double value x instead of the original Python object arg?

Thanks for your suggestion! I forgot this PR while waiting for others to review it, and it’s time to pick it up :)

IMO, it is better to use the C double variable. The only drawback is that we will lose some floating point precision in error message. Does this confuse others? Perhaps this minor flaw is acceptable.

# Using double(%f), the default value is six decimal places.
>>> math.sqrt(-math.pi)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math.sqrt() expects a nonnegative input, got '-3.141593'.
See cmath.sqrt() for a variation that supports complex numbers
>>> math.pi
3.141592653589793

>>> math.sqrt(-0.00000001)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math.sqrt() expects a nonnegative input, got '-0.000000'.
See cmath.sqrt() for a variation that supports complex numbers

AA-Turner · 2023-09-26T09:52:31Z

ValueError: math.sqrt() expects a nonnegative input, got '-3.141593'.

Perhaps just append ' (rounded)' to the end of the line?

A

skirpichev · 2023-09-26T10:14:09Z

In most cases functions in the module are 1-arg. I think that if we show the function domain (e.g. math.sqrt() expects a non-negative input.) - this will be more than enough.

serhiy-storchaka · 2023-09-26T10:52:33Z

Of course, the output should be more precise and consistent with Python representation, so do not use snprintf() or like. There are several ways to do this:

Create a Python float object with PyFloat_FromDouble, then use %R format.
Use PyOS_double_to_string() (as in the implementation of float.__repr__), then use %s format to insert it in the error message.

Before writing code, wait to see if other core developers (in particular @mdickinson and @tim-one) have something to say about this idea.

skirpichev · 2024-07-21T09:52:30Z

FYI, @CharlieZhao95, this branch has unresolved conflicts. Are you planing to finish this? If not, I'll open a pr, based on this work.

CharlieZhao95 · 2024-07-22T07:25:56Z

FYI, @CharlieZhao95, this branch has unresolved conflicts. Are you planing to finish this? If not, I'll open a pr, based on this work.

Thanks for reminding! BTW, is it time to continue this PR? Maybe we can make this PR simpler (remove got '%R' from the error message first)

picnixz

Can you add tests for checking the new messages?

Misc/NEWS.d/next/Library/2023-02-01-16-41-31.gh-issue-101410.Dt2aQE.rst

Modules/mathmodule.c

picnixz · 2024-07-22T07:33:03Z

Modules/mathmodule.c

@@ -2266,11 +2284,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'.");


I don't think using quotes around numbers is relevant. For instance, if it's a custom class of a float that overrides __repr__, then you don't necessarily want to surround the __repr__ by quotes.

Suggested change

num = loghelper(args[0], m_log, "math.log() expects a positive input, got '%R'.");

num = loghelper(args[0], m_log, "math.log() expects a positive input, got %R.");

picnixz · 2024-07-22T07:33:08Z

Modules/mathmodule.c

@@ -2300,7 +2318,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'.");


picnixz · 2024-07-22T07:33:40Z

Modules/mathmodule.c

@@ -2317,7 +2335,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'.");


…t2aQE.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

CharlieZhao95 · 2024-07-22T08:05:44Z

Can you add tests for checking the new messages?

Sure :) I'm waiting for other core developers to reply. I will add tests after they finalize the solution.

skirpichev · 2024-10-15T04:37:03Z

This work continued in #124299

CharlieZhao95 added 3 commits February 1, 2023 11:02

Customize error messages in the math module.

85edd15

Imporve error messages for math.log().

2448fe4

Add a NEWS entry.

c0c1e2d

bedevere-bot added the awaiting review label Feb 1, 2023

bedevere-bot mentioned this pull request Feb 1, 2023

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

Open

add new line

1a491f2

OTheDev reviewed Feb 2, 2023

View reviewed changes

CharlieZhao95 marked this pull request as ready for review February 3, 2023 02:19

mdickinson reviewed Feb 5, 2023

View reviewed changes

Modules/mathmodule.c Outdated Show resolved Hide resolved

CharlieZhao95 added 2 commits February 6, 2023 10:00

fix nit

4cd75b0

Merge remote-tracking branch 'upstream/main' into pythongh-101410/mat…

611dedb

…h_error_message

skirpichev reviewed Feb 18, 2023

View reviewed changes

serhiy-storchaka reviewed Sep 23, 2023

View reviewed changes

Merge branch 'main' into pythongh-101410/math_error_message

ef68c24

Merge branch 'main' into pythongh-101410/math_error_message

dc8e593

picnixz reviewed Jul 22, 2024

View reviewed changes

CharlieZhao95 and others added 3 commits July 22, 2024 15:43

Update Misc/NEWS.d/next/Library/2023-02-01-16-41-31.gh-issue-101410.D…

75a7c5c

…t2aQE.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Modules/mathmodule.c

4b2ec23

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Modules/mathmodule.c

3fc73e9

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Merge branch 'main' into pythongh-101410/math_error_message

30be617

skirpichev mentioned this pull request Sep 21, 2024

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

Merged

skirpichev closed this Oct 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-101410: Improve error messages in math module #101492

gh-101410: Improve error messages in math module #101492

CharlieZhao95 commented Feb 1, 2023 •

edited by bedevere-bot

Loading

OTheDev Feb 2, 2023

CharlieZhao95 Feb 3, 2023

CharlieZhao95 commented Feb 3, 2023

skirpichev Feb 18, 2023

CharlieZhao95 Feb 20, 2023

skirpichev Feb 20, 2023

skirpichev Sep 26, 2023

serhiy-storchaka left a comment

serhiy-storchaka commented Sep 23, 2023 •

edited

Loading

CharlieZhao95 commented Sep 26, 2023

AA-Turner commented Sep 26, 2023 •

edited

Loading

skirpichev commented Sep 26, 2023

serhiy-storchaka commented Sep 26, 2023

skirpichev commented Jul 21, 2024

CharlieZhao95 commented Jul 22, 2024

picnixz left a comment

picnixz Jul 22, 2024

picnixz Jul 22, 2024

picnixz Jul 22, 2024

CharlieZhao95 commented Jul 22, 2024

skirpichev commented Oct 15, 2024

	num = loghelper(args[0], m_log, "math.log() expects a positive input, got '%R'.");
	num = loghelper(args[0], m_log, "math.log() expects a positive input, got %R.");

gh-101410: Improve error messages in math module #101492

gh-101410: Improve error messages in math module #101492

Conversation

CharlieZhao95 commented Feb 1, 2023 • edited by bedevere-bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CharlieZhao95 commented Feb 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhiy-storchaka left a comment

Choose a reason for hiding this comment

serhiy-storchaka commented Sep 23, 2023 • edited Loading

CharlieZhao95 commented Sep 26, 2023

AA-Turner commented Sep 26, 2023 • edited Loading

skirpichev commented Sep 26, 2023

serhiy-storchaka commented Sep 26, 2023

skirpichev commented Jul 21, 2024

CharlieZhao95 commented Jul 22, 2024

picnixz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CharlieZhao95 commented Jul 22, 2024

skirpichev commented Oct 15, 2024

CharlieZhao95 commented Feb 1, 2023 •

edited by bedevere-bot

Loading

serhiy-storchaka commented Sep 23, 2023 •

edited

Loading

AA-Turner commented Sep 26, 2023 •

edited

Loading