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.

>>>importmath# before>>>math.sqrt(-1) Traceback (mostrecentcalllast): File"<stdin>", line1, in<module>ValueError: mathdomainerror# now>>>math.sqrt(-1) Traceback (mostrecentcalllast): File"<stdin>", line1, in<module>ValueError: math.sqrt() expectsanon-negativeinput, got'-1'. Seecmath.sqrt() forvariationthatsupportscomplexnumbers>>>math.log(0) Traceback (mostrecentcalllast): File"<stdin>", line1, in<module>ValueError: math.log() expectsapositiveinput, 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

 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 (mostrecentcalllast): File"<stdin>", line1, in<module>ValueError: math.sqrt() expectsanonnegativeinput, got'-3.141593'. Seecmath.sqrt() foravariationthatsupportscomplexnumbers>>>math.pi3.141592653589793>>>math.sqrt(-0.00000001) Traceback (mostrecentcalllast): File"<stdin>", line1, in<module>ValueError: math.sqrt() expectsanonnegativeinput, got'-0.000000'. Seecmath.sqrt() foravariationthatsupportscomplexnumbers

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

 returnNULL;

-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

 /*[clinic end generated code: output=5425899a4d5d6acb input=08321262bae4f39b]*/
 {
-returnloghelper(x, m_log2);
+returnloghelper(x, m_log2, "math.log2() expects a positive input, got '%R'.");


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

Modules/mathmodule.c

 /*[clinic end generated code: output=be72a64617df9c6f input=b2469d02c6469e53]*/
 {
-returnloghelper(x, m_log10);
+returnloghelper(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
Closed

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 resolvedHide resolved

CharlieZhao95 added 2 commits February 6, 2023 10:00

fix nit
4cd75b0

Merge remote-tracking branch 'upstream/main' intopythongh-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' intopythongh-101410/math_error_message
ef68c24

Merge branch 'main' intopythongh-101410/math_error_message
dc8e593

picnixz reviewed Jul 22, 2024
View reviewed changes

CharlieZhao95and 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' intopythongh-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

skirpichev mentioned this pull request May 12, 2025
gh-133904: math.factorial documentation fix #133907
Merged

	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.");

Uh oh!

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

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

Uh oh!

Conversation

CharlieZhao95 commented Feb 1, 2023• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CharlieZhao95 commented Feb 3, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Sep 23, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CharlieZhao95 commented Sep 26, 2023

Uh oh!

AA-Turner commented Sep 26, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev commented Sep 26, 2023

Uh oh!

serhiy-storchaka commented Sep 26, 2023

Uh oh!

skirpichev commented Jul 21, 2024

Uh oh!

CharlieZhao95 commented Jul 22, 2024

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CharlieZhao95 commented Jul 22, 2024

Uh oh!

skirpichev commented Oct 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

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