Skip to content

gh-99553: add tests for ExceptionGroup wrapping #99615

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hauntsaninja merged 6 commits into from
Apr 11, 2023
Merged

gh-99553: add tests for ExceptionGroup wrapping #99615

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4b9e01a
gh-99553: clarify doc and add tests
Zac-HD Nov 20, 2022
29b9f27
Update Doc/library/exceptions.rst
Zac-HD Nov 20, 2022
07bdfea
Update Doc/library/exceptions.rst
Zac-HD Nov 21, 2022
9bdb9c6
undo docs changes
hauntsaninja Apr 11, 2023
774b0b8
Merge branch 'main' into zac/custom-exceptiongroup-docs-tests
hauntsaninja Apr 11, 2023
043d002
Merge branch 'main' into zac/custom-exceptiongroup-docs-tests
hauntsaninja Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update Doc/library/exceptions.rst 
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
  • Loading branch information
@Zac-HD @hauntsaninja
Zac-HD and hauntsaninja authored Nov 20, 2022
commit 29b9f276dc0896dc282b45d680ebb93a4403f866
6 changes: 3 additions & 3 deletions Doc/library/exceptions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,9 +965,9 @@ their subgroups based on the types of the contained exceptions.
def derive(self, excs):
return Errors(excs, self.exit_code)

Subclasses of :exc:`BaseExceptionGroup` cannot wrap :exc:`BaseException`\ s
if they extend :exc:`Exception`, as for :exc:`ExceptionGroup`. However,
there is no further restriction: a :exc:`BaseExceptionGroup` subclass
A subclass of :exc:`BaseExceptionGroup` that extends :exc:`Exception`
cannot wrap :exc:`BaseException`\ s, as is the case with :exc:`ExceptionGroup`.
Comment thread
Zac-HD marked this conversation as resolved.
Outdated
However, there is no further restriction: a :exc:`BaseExceptionGroup` subclass
which extends :exc:`KeyError` could wrap any :exc:`Exception`.
Copy link
Copy Markdown
Member

@iritkatriel iritkatriel Nov 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a strange thing to say in a doc, because why would you expect it to not wrap any Exception?

Maybe instead of the second sentence we could just explain the motivation for the restriction: “This ensures that wrapped BaseExceptions are not caught by ‘except Exception’.”

Copy link
Copy Markdown
Contributor Author

@Zac-HD Zac-HD Nov 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would you expect it to not wrap any Exception?

Suppose we have class MyEG(BaseExceptionGroup, X):, where except X: will catch such a group because it extends X. We disallow wrapping BaseExceptions in an ExceptionGroup because they would not be caught by except Exception: if unwrapped.

However this logic is not applied consistently: if X is e.g. KeyError, then except KeyError: can indeed catch MyEG("", [ValueError()]). I find this inconsistency surprising, which is why I commented on #99572 and then opened this PR!

(I'm not trying to reopen the design discussion, just trying to explain my perspective)

Copy link
Copy Markdown
Member

@iritkatriel iritkatriel Nov 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see. Well yes, the Exception case is different - people use ‘expect Exception’ as ‘catch everything except fatal errors’.


.. versionadded:: 3.11
Expand Down