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-103590: do not wrap a single exception raised from a try-except* by iritkatriel · Pull Request #103665 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Apr 21, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Fixes #103590.

  • Issue: [ExceptionGroup] Inconsistent raise inside except* #103590

Sorry, something went wrong.

Uh oh!

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

All reactions
@iritkatriel
pythongh-103590: do not wrap a single exception raised from a try-exc…
17a1311
…ept*
@bedevere-bot bedevere-bot mentioned this pull request Apr 21, 2023
[ExceptionGroup] Inconsistent raise inside except* #103590
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Apr 21, 2023
@blurb-it
📜🤖 Added by blurb_it.
6504e9b
@iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) needs backport to 3.11 only security fixes labels Apr 21, 2023
@iritkatriel
iritkatriel requested a review from gvanrossum April 21, 2023 16:15
@iritkatriel iritkatriel added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 21, 2023
@bedevere-bot

bedevere-bot commented Apr 21, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 6504e9b 🤖

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 Apr 21, 2023
@iritkatriel
Merge branch 'main' into wrapping
c4ffc7b
@iritkatriel
iritkatriel requested a review from ambv April 24, 2023 20:03
@ambv ambv added DO-NOT-MERGE and removed DO-NOT-MERGE labels Apr 24, 2023
@ambv
Merge branch 'main' into wrapping
3e8f08c
gvanrossum
gvanrossum reviewed Apr 25, 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.

Looking at the logic and the test changes I wonder if this is a good idea?

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Misc/NEWS.d/next/Core and Builtins/2023-04-21-16-12-41.gh-issue-103590.7DHDOE.rst Resolved

Uh oh!

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

@iritkatriel

iritkatriel commented Apr 25, 2023

Copy link
Copy Markdown
Member Author

Looking at the logic and the test changes I wonder if this is a good idea?

The example on the issue looks pretty convincing: #103590 (comment)

ambv reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@iritkatriel iritkatriel removed the needs backport to 3.11 only security fixes label Apr 25, 2023
@iritkatriel
Merge branch 'main' into wrapping
34bac84
@gvanrossum

gvanrossum commented Apr 26, 2023

Copy link
Copy Markdown
Member

The example on the issue looks pretty convincing: #103590 (comment)

I find that comment a bit confusing, but a later comment by Carl is indeed convincing.

LGTM.

All reactions

Sorry, something went wrong.

Uh oh!

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

gvanrossum
gvanrossum approved these changes Apr 26, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Apr 26, 2023
iritkatriel and others added 4 commits April 26, 2023 22:07
@iritkatriel
add to whatsnew
c3cdf7c
@iritkatriel
add issue ref to whatsnew
a557024
@iritkatriel
whitespace
6e22c91
@iritkatriel
Merge branch 'main' into wrapping
e11ab45
gvanrossum
gvanrossum approved these changes Apr 26, 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.

+1

Sorry, something went wrong.

Uh oh!

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

All reactions
iritkatriel
iritkatriel commented Apr 26, 2023
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

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

@iritkatriel
typo
78711e1
@iritkatriel
iritkatriel merged commit 63842bd into python:main Apr 27, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 27, 2023
@gvanrossum

gvanrossum commented May 2, 2023

Copy link
Copy Markdown
Member

Should this be backported to 3.11?

All reactions

Sorry, something went wrong.

Uh oh!

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

@iritkatriel

iritkatriel commented May 2, 2023

Copy link
Copy Markdown
Member Author

Should this be backported to 3.11?

I thought so, but then I got the impression that you and @ambv thought it shouldn't.

All reactions

Sorry, something went wrong.

Uh oh!

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

@gvanrossum

gvanrossum commented May 2, 2023

Copy link
Copy Markdown
Member

So I'm changing my mind, there can't be too much code out there that would be broken by this fix. What does @ambv think?

All reactions

Sorry, something went wrong.

Uh oh!

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

@ambv

ambv commented May 2, 2023

Copy link
Copy Markdown
Contributor

This is a bug. I agree there is probably little code out there that will be affected by the difference.

However, it's somewhat of a subtle change so user code is unlikely to be tested against it, and if a difference is experienced, the error displayed might be surprising. I'm thinking: "it works on my local box but on CI there's an exception, both are 3.11". While we continue improving 3.11, some installations will be forever stuck at 3.11.1 or so.

But given 3.11's low adoption so far I tend to agree that backporting is probably harmless and ultimately more helpful.

Let's backport then. But let's also be explicit about the change in the docs then. 3.12 whatsnew suggests this change only made it to 3.12, we need to amend that with a mention like "also changed in 3.11.4", and mention this in the except-star docs for both releases.

All reactions

Sorry, something went wrong.

Uh oh!

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

@gvanrossum

gvanrossum commented May 2, 2023

Copy link
Copy Markdown
Member

Good plan. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

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

iritkatriel added a commit to iritkatriel/cpython that referenced this pull request May 2, 2023
@iritkatriel
pythongh-103590: do not wrap a single exception raised from a try-exc…
2713e5a
…ept* (python#103665)
@iritkatriel iritkatriel mentioned this pull request May 2, 2023
[3.11] gh-103590: do not wrap a single exception raised from a try-except* #104094
Merged
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
@ambv ambv Awaiting requested review from ambv

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) type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ExceptionGroup] Inconsistent raise inside except*

Uh oh!

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

4 participants

@iritkatriel @bedevere-bot @gvanrossum @ambv
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.