Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-103791: Make contextlib.suppress also act on exceptions within an ExceptionGroup#103792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…in an ExceptionGroup
| match, rest = excinst.split(self._exceptions) | ||
| if rest is None: | ||
| return True | ||
| raise rest |
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 isn't ideal as it makes the exception group's own traceback include thedef __exit__(...) frame. For example:
File "/Volumes/RAMDisk/cpython/Lib/test/test_contextlib.py", line 1220, in test_exception_groups with suppress(ValueError): File "/Volumes/RAMDisk/cpython/Lib/contextlib.py", line 454, in __exit__ raise rest from excinst File "/Volumes/RAMDisk/cpython/Lib/test/test_contextlib.py", line 1221, in test_exception_groups raise eg_all()in case of the newly added test.
This isn't a big problem because:
- the group's member exceptions contain pristine tracebacks, only the group itself contains extra frames; and
- the
raiseof the original group is still included in the traceback because we used.split()to create the new object.
Ideally, we wouldn't need this. However, the API of__exit__ makes it impossible to replace theExceptionGroup instance with another one, whileExceptionGroup itself makes its exceptions read-only.
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.
LGTM. Minor doc tweaks.
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Library/2023-04-24-23-07-56.gh-issue-103791.bBPWdS.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
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.
(just testing out "request changes")
bedevere-bot commentedApr 24, 2023
When you're done making the requested changes, leave the comment: |
OK, GitHub indeed doesn't disable auto-merge when changes are requested
* main:pythongh-87729: add LOAD_SUPER_ATTR instruction for faster super() (python#103497)pythongh-103791: Make contextlib.suppress also act on exceptions within an ExceptionGroup (python#103792)
| return | ||
| if issubclass(exctype, self._exceptions): | ||
| return True | ||
| if issubclass(exctype, ExceptionGroup): |
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.
Was there a specific reason to handleExceptionGroup but notBaseExceptionGroup?
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 doubt it. Want to make a PR?
Uh oh!
There was an error while loading.Please reload this page.