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-102594: PyErr_SetObject adds note to exception raised on normalization error by iritkatriel · Pull Request #102675 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Mar 14, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Fixes #102594.

Co-authored-by: Ralf W. Grosse-Kunstleve [email protected]

  • Issue: Suspected PyErr_Fetch() behavior change #102594

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iritkatriel
PyErr_SetObject adds note to exception raised on normalization error
ff03c61
@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 14, 2023
@iritkatriel
iritkatriel requested review from gvanrossum and markshannon March 14, 2023 00:09
@bedevere-bot bedevere-bot mentioned this pull request Mar 14, 2023
Suspected PyErr_Fetch() behavior change #102594
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Mar 14, 2023
blurb-it Bot and others added 2 commits March 14, 2023 00:11
@blurb-it
📜🤖 Added by blurb_it.
31e6415
@iritkatriel
Merge branch 'main' into setobject-note
8d6285c
@erlend-aasland

erlend-aasland commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

I was under the impression that we should not use exception notes internally. I'll try to find a link to the previous discussion (between Victor and Irit, IIRC); it may be relevant to this change.
All reactions

Sorry, something went wrong.

Uh oh!

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

iritkatriel commented Mar 14, 2023

Copy link
Copy Markdown
Member Author

I was under the impression that we should not use exception notes internally. I'll try to find a link to the previous discussion (between Victor and Irit, IIRC); it may be relevant to this change.

There is no general ban on using the notes in the interpreter. The discussion you are referring to was about splitting the message into a "core error message" as arg and "explanation" on the note, so that IDEs can toggle the explanation on and off. The objection was to introduce assumptions about what is in the note, and how IDEs should handle it. There is also a general guideline that information which is known at the time the exception object is constructed does not belong on the note. Technically of course you can put it on the note, but that was not their intended use.

This PR addresses the classic use case for PEP-678: we have an exception that we did not construct, and we want to enrich it with additional information which was not available at the time that the exception was constructed.
erlend-aasland and Zac-HD reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
add missing 'static'
827a48c
@erlend-aasland

erlend-aasland commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

Yeah, that discussion sounds familiar. Do you happen to have a link to it?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel
iritkatriel commented Mar 14, 2023
View reviewed changes
Comment thread Python/errors.c Resolved

Uh oh!

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

iritkatriel commented Mar 14, 2023

Copy link
Copy Markdown
Member Author

Yeah, that discussion sounds familiar. Do you happen to have a link to it?

I don't, sorry.
All reactions

Sorry, something went wrong.

Uh oh!

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

iritkatriel commented Mar 14, 2023

Copy link
Copy Markdown
Member Author

FYI @Zac-HD
All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood commented Mar 14, 2023

Copy link
Copy Markdown
Member

Yeah, that discussion sounds familiar. Do you happen to have a link to it?

I don't, sorry.

There was some previous discussion in:

  • gh-95778: Use a note for the max digits error message #96878
  • Use Python 3.11 exception notes (PEP 678) to provide hints on errors #96958
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum
gvanrossum reviewed Mar 14, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment

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.

(We discussed this extensively offline, but here are some nits I had already found.)

FWIW I think this is an excellent use case for notes, unlike the two issues Alex linked to.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_capi/test_exceptions.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_capi/test_exceptions.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel and others added 2 commits March 14, 2023 19:49
@iritkatriel
apply offline comments from Guido
4333933
@iritkatriel
Merge branch 'main' into setobject-note
b3448fd
gvanrossum
gvanrossum approved these changes Mar 14, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment

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.

LG, just one optional suggestion.

Sorry, something went wrong.

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Mar 14, 2023
iritkatriel and others added 2 commits March 14, 2023 21:59
@iritkatriel
use Py_XSETREF
03dbc6b
@iritkatriel
Merge branch 'main' into setobject-note
8f0f2be
@iritkatriel iritkatriel added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 14, 2023
@bedevere-bot

bedevere-bot commented Mar 14, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 8f0f2be 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 14, 2023
gvanrossum
gvanrossum approved these changes Mar 14, 2023
View reviewed changes
iritkatriel and others added 2 commits March 15, 2023 10:17
@iritkatriel
fix whitespace
a9d1068
@iritkatriel
Merge branch 'main' into setobject-note
e834a8f
AlexWaygood and others added 3 commits March 15, 2023 18:25
@AlexWaygood
Merge branch 'main' into setobject-note
a1a7d78
@iritkatriel
Merge branch 'main' into setobject-note
ed90b66
@iritkatriel
Merge branch 'main' into setobject-note
140770f
@iritkatriel iritkatriel added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 16, 2023
@bedevere-bot

bedevere-bot commented Mar 16, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 140770f 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 16, 2023
@iritkatriel
iritkatriel merged commit 51d693c into python:main Mar 16, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 16, 2023
@rwgk rwgk mentioned this pull request Mar 16, 2023
ci: Python 3.12 optional test job pybind/pybind11#4575
Merged
@gvanrossum

gvanrossum commented Mar 16, 2023

Copy link
Copy Markdown
Member

PS. Shouldn't PyErr_NormalizeException also add the same note?
All reactions

Sorry, something went wrong.

Uh oh!

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

iritkatriel commented Mar 16, 2023

Copy link
Copy Markdown
Member Author

PS. Shouldn't PyErr_NormalizeException also add the same note?

We could, but it's deprecated so I'm not sure how much it matters now.
All reactions

Sorry, something went wrong.

Uh oh!

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

gvanrossum commented Mar 16, 2023

Copy link
Copy Markdown
Member

Fair
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
carljm added a commit to carljm/cpython that referenced this pull request Mar 17, 2023
@carljm
Merge branch 'main' into inlinecomp2
1a8f4a0 
* main: (34 commits)
  pythongh-102701: Fix overflow in dictobject.c (pythonGH-102750)
  pythonGH-78530: add support for generators in `asyncio.wait` (python#102761)
  Increase stack reserve size for Windows debug builds to avoid test crashes (pythonGH-102764)
  pythongh-102755: Add PyErr_DisplayException(exc) (python#102756)
  Fix outdated note about 'int' rounding or truncating (python#102736)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (python#102760)
  pythongh-99726: Improves correctness of stat results for Windows, and uses faster API when available (pythonGH-102149)
  pythongh-102192: remove redundant exception fields from ssl module socket (python#102466)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (python#102743)
  pythongh-102737: Un-ignore ceval.c in the CI globals check (pythongh-102745)
  pythonGH-102748: remove legacy support for generator based coroutines from `asyncio.iscoroutine` (python#102749)
  pythongh-102721: Improve coverage of `_collections_abc._CallableGenericAlias` (python#102722)
  pythonGH-102653: Make recipe docstring show the correct distribution (python#102742)
  Add comments to `{typing,_collections_abc}._type_repr` about each other (python#102752)
  pythongh-102594: PyErr_SetObject adds note to exception raised on normalization error (python#102675)
  pythongh-94440: Fix issue of ProcessPoolExecutor shutdown hanging (python#94468)
  pythonGH-100112:  avoid using iterable coroutines in asyncio internally (python#100128)
  pythongh-102690: Use Edge as fallback in webbrowser instead of IE (python#102691)
  pythongh-102660: Fix Refleaks in import.c (python#102744)
  pythongh-102738: remove from cases generator the code related to register instructions (python#102739)
  ...
@iritkatriel
iritkatriel deleted the setobject-note branch March 19, 2023 11:50
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
@iritkatriel @Fidget-Spinner
pythongh-102594: PyErr_SetObject adds note to exception raised on nor…
a965c5e 
…malization error (python#102675)
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@iritkatriel @warsaw
pythongh-102594: PyErr_SetObject adds note to exception raised on nor…
807a421 
…malization error (python#102675)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes
@markshannon markshannon Awaiting requested review from markshannon

Uh oh!

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

Assignees

No one assigned

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Suspected PyErr_Fetch() behavior change

Uh oh!

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

5 participants

@iritkatriel @erlend-aasland @AlexWaygood @bedevere-bot @gvanrossum
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.