GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-133895: provide C99 Annex F return values for math's functions by skirpichev · Pull Request #135008 · python/cpython

Conversation

@skirpichev

@skirpichev skirpichev commented Jun 1, 2025 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Now this information is available as the "value" attribute of the ValueError exception object:

>>> import math
>>> try:
...     math.gamma(-0.0)
... except ValueError as exc:
...     print(exc.value)
...
-inf
  • Issue: Pass return value on ValueError exceptions in the cmath/math modules #133895

📚 Documentation preview 📚: https://cpython-previews--135008.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@skirpichev
pythongh-133895: provide C99 Annex F return values for math functions
2bfdc0a 
Now this information is available as the "value" attribute of the
ValueError exception object:

```pycon
>>> import math
>>> try:
...     math.gamma(-0.0)
... except ValueError as exc:
...     print(exc.value)
...
-inf
```
@bedevere-app bedevere-app Bot added the awaiting review label Jun 1, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jun 1, 2025
Pass return value on ValueError exceptions in the cmath/math modules #133895
Open
@skirpichev

skirpichev commented Jun 1, 2025

Copy link
Copy Markdown
Member Author

See also #134995
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@skirpichev skirpichev changed the title gh-133895: provide C99 Annex F return values for math functions gh-133895: provide C99 Annex F return values for math's functions Jun 1, 2025
picnixz
picnixz reviewed Jun 1, 2025
View reviewed changes
Comment thread Doc/whatsnew/3.15.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Library/2025-06-01-16-10-07.gh-issue-133895.1-SE2w.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/mathmodule.c
PyObject *value = PyFloat_FromDouble(x);

if (value) {
PyObject_SetAttrString(exc, "value", value);

@picnixz picnixz Jun 1, 2025

Copy link
Copy Markdown
Member

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.

This call can fail. Should we do something about it? or should we simply ignore the exception? or maybe chain it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@skirpichev skirpichev Jun 1, 2025

Copy link
Copy Markdown
Member Author

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.

or should we simply ignore the exception?

I don't see better option so far. (Ditto for cmath.)

BTW, maybe I should also drop if (value) { clause.

or maybe chain it?

I doubt it's appropriate here. We don't handle ValueError here. This code just alter the raised exception object.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@picnixz picnixz Jun 1, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

BTW, maybe I should also drop if (value) { clause.

I don't think you should, otherwise PyObject_SetAttrString will be called with an exception set (and with a NULL value it's the same as deleting the attribute)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@skirpichev skirpichev Jun 1, 2025

Copy link
Copy Markdown
Member Author

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.

will be called with an exception set

Why it's bad in this case?

and with a NULL value it's the same as deleting the attribute

But we are already ignore errors from the PyObject_SetAttrString :-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@picnixz picnixz Jun 1, 2025

Copy link
Copy Markdown
Member

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.

Why it's bad in this case?

Don't we have an assert() somewhere that would fail if the error indicator is set?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@skirpichev skirpichev Jun 2, 2025

Copy link
Copy Markdown
Member Author

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 failed to find such case.

Though, we could explicitly call (twice) PyErr_Occurred() in appropriate places.

Looking on other cases of using PyErr_GetRaisedException() - I think that unraisable exceptions should be used here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@skirpichev skirpichev Jun 2, 2025

Copy link
Copy Markdown
Member Author

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 cmath's pr, I did such change: #134995. Does it make sense for you?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@skirpichev @picnixz
Apply suggestions from code review
c1eb063 
Co-authored-by: Bénédikt Tran <[email protected]>
@skirpichev
skirpichev marked this pull request as draft June 2, 2025 10:30
@bedevere-app bedevere-app Bot removed the awaiting review label Jun 2, 2025
skirpichev added 2 commits June 24, 2025 17:28
@skirpichev
Merge branch 'main' into math-ValueError/133895
df077cb
@skirpichev
Merge branch 'main' into math-ValueError/133895
64a0018
@skirpichev skirpichev closed this Oct 10, 2025
@skirpichev
skirpichev deleted the math-ValueError/133895 branch October 10, 2025 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz left review comments
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner will be requested when the pull request is marked ready for review AA-Turner is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@skirpichev @picnixz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.