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 — added exception groups by iritkatriel · Pull Request #25430 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Apr 16, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

First step of PEP-654: added Exception Groups (without updating traceback formatting).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iritkatriel
iritkatriel requested a review from a team as a code owner April 16, 2021 00:04
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Apr 16, 2021
@bedevere-bot bedevere-bot added the awaiting review label Apr 16, 2021
@iritkatriel iritkatriel added the DO-NOT-MERGE label Apr 16, 2021
@iritkatriel
iritkatriel requested review from 1st1 and gvanrossum April 16, 2021 00:05
iritkatriel
iritkatriel commented Apr 16, 2021
View reviewed changes
Comment thread Include/pyerrors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/exception_hierarchy.txt Outdated

@iritkatriel iritkatriel Apr 16, 2021

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.

Unfortunately we can't express here that ExceptionGroup subclasses BaseExceptionGroup as well.

Sorry, something went wrong.

Uh oh!

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

@gvanrossum gvanrossum Apr 16, 2021

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.

Add a note in parentheses?

Sorry, something went wrong.

Uh oh!

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

@iritkatriel iritkatriel Apr 16, 2021

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.

There seems to be an unused feature that interprets whatever is in "(..)" as a platform name and then ignores any exception where the platform is not the current platform (so it won't break the test but will weaken it).

But "[...]" seems ok - anything in them appears to be ignored, and when I changed ExceptionGroup to something invalid the test failed.

cpython/Lib/test/test_baseexception.py

Line 49 in 8bbfeb3

if '(' in exc_name:

Sorry, something went wrong.

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum
gvanrossum reviewed Apr 16, 2021
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.

Is this what we will merge once the PEP is accepted?

(Note that the master -> main rename will happen at that time too. But supposedly PSs will be automatically adjusted. We’ll see.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/pyerrors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/exception_hierarchy.txt Outdated

@gvanrossum gvanrossum Apr 16, 2021

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.

Add a note in parentheses?

Sorry, something went wrong.

Uh oh!

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

gvanrossum commented Apr 16, 2021

Copy link
Copy Markdown
Member

Probably mark as Draft?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
iritkatriel marked this pull request as draft April 16, 2021 07:30
@iritkatriel
iritkatriel removed the request for review from a team April 16, 2021 09:43
@iritkatriel
iritkatriel force-pushed the PEP-654-ExceptionGroup branch from bbedd10 to 40c3cd0 Compare April 16, 2021 14:03
iritkatriel
iritkatriel commented Apr 16, 2021
View reviewed changes

@iritkatriel iritkatriel left a comment

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.

Is this what we will merge once the PEP is accepted?

That's the idea. The branch of the other PR is very old and compile.c/eval.c changed beyond what can be fixed with a rebase. So I started from scratch on a recent cut of master. Marked as draft for now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/exception_hierarchy.txt Outdated

@iritkatriel iritkatriel Apr 16, 2021

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.

There seems to be an unused feature that interprets whatever is in "(..)" as a platform name and then ignores any exception where the platform is not the current platform (so it won't break the test but will weaken it).

But "[...]" seems ok - anything in them appears to be ignored, and when I changed ExceptionGroup to something invalid the test failed.

cpython/Lib/test/test_baseexception.py

Line 49 in 8bbfeb3

if '(' in exc_name:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/pyerrors.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel
iritkatriel force-pushed the PEP-654-ExceptionGroup branch from 81b6b08 to cecdf32 Compare April 20, 2021 18:42
@gvanrossum

gvanrossum commented May 11, 2021

Copy link
Copy Markdown
Member

You should probably merge this on main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel added 7 commits May 11, 2021 23:14
@iritkatriel
added exception groups
af3c3f7
@iritkatriel
fix build on mac/ubuntu
c1d592f
@iritkatriel
+-- ExceptionGroup [BaseExceptionGroup] in exception_hierarchy.txt
187b10d
@iritkatriel
regenerated Python/importlib_external.h
a6d6d71
@iritkatriel
fix smelly symbol and whitespace
9ffd6e5
@iritkatriel
detect matcher type only once. Add option to match by instance(s)-equ…
5f05645 
…ality which is needed for cycle detection and except*'s reraise logic
@iritkatriel
regenerated Python/importlib_external.h
0a9e6b5
@iritkatriel
iritkatriel force-pushed the PEP-654-ExceptionGroup branch from cecdf32 to 0a9e6b5 Compare May 11, 2021 23:33
@iritkatriel

iritkatriel commented May 11, 2021

Copy link
Copy Markdown
Member Author

I rebased and regenerated Python/importlib_external.h
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel added 3 commits May 12, 2021 10:37
@iritkatriel
regenerated PC/python3dll.c
aed5cff
@iritkatriel
updated Doc/data/stable_abi.dat and regenerated PC/python3dll.c, Doc/…
291be68 
…data/stable_abi.dat
@iritkatriel
updated Misc/stable_abi.txt
5b3dfe2
@github-actions

github-actions Bot commented Jun 12, 2021

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Jun 12, 2021
@iritkatriel

iritkatriel commented Sep 26, 2021

Copy link
Copy Markdown
Member Author

Superseded by #28569.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel iritkatriel closed this Sep 26, 2021
@iritkatriel
iritkatriel deleted the PEP-654-ExceptionGroup branch October 18, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum left review comments
@1st1 1st1 Awaiting requested review from 1st1
@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon will be requested when the pull request is marked ready for review brettcannon is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently will be requested when the pull request is marked ready for review ericsnowcurrently is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan will be requested when the pull request is marked ready for review ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw will be requested when the pull request is marked ready for review warsaw is a code owner
@FFY00 FFY00 Awaiting requested review from FFY00 FFY00 will be requested when the pull request is marked ready for review FFY00 is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

awaiting review DO-NOT-MERGE stale Stale PR or inactive for long period of time.

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.

4 participants

@iritkatriel @gvanrossum @the-knights-who-say-ni @bedevere-bot
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.