Accept generic ExceptionGroups for raises #13134
Conversation
tapetersen
changed the title
psf-chronographer
bot
added
the
bot:chronographer:provided
src/_pytest/python_api.py
Outdated
| if issubclass(origin_exc, ExceptionGroup) and exc_type is Exception: | ||
| return cast(type[E], origin_exc) | ||
| elif ( | ||
| issubclass(origin_exc, BaseExceptionGroup) and exc_type is BaseException |
There was a problem hiding this comment.
I'd be inclined to do ... exc_type in (Any, (Base)Exception): here - the escape-hatch is particularly useful when you might want to do something specific with the contents later.
There was a problem hiding this comment.
Sure, fixed in patch (left as own commit I assume you'll squash)
tapetersen
force-pushed
the
main
branch
2 times, most recently
from
c2a9b93 to
a74e557
Compare
|
Thanks for the review and consideration. |
There was a problem hiding this comment.
Thanks @tapetersen for the contribution!
I took the liberty of pushing some small changes that I noticed while testing this locally.
Let's please squash this when getting around to merge it. 👍
Ahh yes the traceback machinery you have I forgot about. If you find other things feel free to fix them directly and I do appreciate reading about why. |
| if isinstance(expected_exception, type): | ||
| expected_exceptions: tuple[type[E], ...] = (expected_exception,) | ||
| expected_exceptions = (expected_exception,) |
There was a problem hiding this comment.
Without a comma it's not a tuple but just a singular value.
Python 3.10.12 (main, Jan 17 2025, 14:35:34) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> (1)
1
>>> (1,)
(1,)
>>>
The original code had the same pattern.
There was a problem hiding this comment.
In fact it's the parenthesis that are superfluous for everything except when creating the empty tuple
>>> 1,
(1,)
src/_pytest/python_api.py
Outdated
| if issubclass(origin_exc, ExceptionGroup) and exc_type in (Exception, Any): | ||
| return cast(type[E], origin_exc) | ||
| elif issubclass(origin_exc, BaseExceptionGroup) and exc_type in ( | ||
| BaseException, |
There was a problem hiding this comment.
I'm not sure if your're refering to the if clauses (which are both needed) or that the action is the same.
If the latter, yes it could be refactored to a single clause with or if you think that's clearer (I tried both but especially after black formatting this looked clearer to me)
There was a problem hiding this comment.
Tried the single clause variation again and I'm ok either way (the previous try was before I added the Any variation as well)
There was a problem hiding this comment.
@nicoddemus Didn't see your comment before trying the alternative. Feel free to rebase and revert/keep as you want.
|
Thanks again everyone! This will be a nice improvement for many people who like detailed static types 🙂 |
Accept
(Base)ExceptionGroupwith generic arguments inpytest.raisesCloses #13115
If this change fixes an issue, please:
closes #XYZWto the PR description and/or commits (whereXYZWis the issue number). See the github docs for more information.Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:
Create a new changelog file in the
changelogfolder, with a name like<ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.Write sentences in the past or present tense, examples:
Also make sure to end the sentence with a
..Add yourself to
AUTHORSin alphabetical order.