Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
ambv merged 6 commits intopython:mainfromambv:gh-103791
Apr 24, 2023

Conversation

@ambv
Copy link
Contributor

@ambvambv commentedApr 24, 2023
edited by bedevere-bot
Loading

watchakorn-18k reacted with thumbs up emoji
match,rest=excinst.split(self._exceptions)
ifrestisNone:
returnTrue
raiserest
Copy link
ContributorAuthor

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
  • theraise of 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.

Copy link
Member

@iritkatrieliritkatriel left a 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.

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@ambvambvenabled auto-merge (squash)April 24, 2023 22:02
hugovk
hugovk previously requested changesApr 24, 2023
Copy link
Member

@hugovkhugovk left a 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")

ambv reacted with thumbs up emoji
@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment:I have made the requested changes; please review again.

@ambvambv dismissedhugovk’sstale reviewApril 24, 2023 22:09

OK, GitHub indeed doesn't disable auto-merge when changes are requested

@ambvambv merged commit22bed58 intopython:mainApr 24, 2023
carljm added a commit to carljm/cpython that referenced this pull requestApr 24, 2023
* 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
ifissubclass(exctype,self._exceptions):
returnTrue
ifissubclass(exctype,ExceptionGroup):
Copy link
Contributor

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?

Copy link
Member

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?

Zac-HD reacted with thumbs up emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@iritkatrieliritkatrieliritkatriel approved these changes

@hugovkhugovkhugovk left review comments

@1st11st1Awaiting requested review from 1st1

+1 more reviewer

@Zac-HDZac-HDZac-HD left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ambv@bedevere-bot@iritkatriel@hugovk@Zac-HD

[8]ページ先頭

©2009-2025 Movatter.jp