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.
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace MultiError with BaseExceptionGroup #2213
Replace MultiError with BaseExceptionGroup #2213
Changes from 92 commits
31706e8
743354b
6fdd0fe
13de11d
9ecca9b
21757c8
61a1646
ca05efb
0b159e8
e9aaabc
83e054c
370e40f
e2fbec0
95a7b9e
745f3cf
bc043aa
797c234
8a09778
c1200de
30358d1
54eb0ce
f1cbcf7
0f5dc79
36e0eba
20df5d8
6eeb137
2dc1c64
e7faa46
c41517a
50963c6
ba17fbb
8911e0a
dfd084c
b3c79c8
81feff2
bcb442b
ab78e02
9abddfa
efc8b01
9a4f512
6cb5ad9
ee245a0
1e36f8c
034bd32
80a751f
9eb2d1c
fd56492
af6233b
a4876bc
5c09f44
c5af0a4
93c81f0
b4791f8
70bb109
1c403b1
a28bfac
7694592
161c0ab
226184f
748ceb8
dfdda5b
d49abbb
a877191
e8a1b60
4bd09bc
1101ead
cd11213
7088be4
5bc2bd9
e87de6c
6b5e4f6
9ff7a45
2140b70
1da1af4
f9e11fc
1d99b23
2591808
afd798f
8c77459
b621e70
656fad4
6e86da3
e29c00f
16c19b6
c35fccc
7b7acb9
3b97ce1
1075ff0
458f5e2
0f39b3f
dc35213
f9eff8e
30b8f3b
232688b
0b8e908
878af56
cc7d76a
a20e2d8
1aba33e
0c283b7
0364b48
bb31ed6
2fd8309
80889b2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 there any documentation about the exact semantics of
catch
? It's not in 3.11 so you can't just defer to the standard library docs.exceptiongroup.readthedocs.io
seems to exist but it's only a stub. It would be nice to be able to link to something definitive here.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.
It's in the README. Let me know if that's not enough.
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.
What exact semantics have been left undocumented there? It would be easier for me to fill in the holes if I knew what they are.
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.
AFAICT basically the sole documentation for catch() semantics is
It is implied that catch() is a drop-in substitute for
except*
, but it's not:except*
passes an ExceptionGroup (ie the result of split()) whilecatch()
calls the handler once for each "leaf" exception (which is a footgun -- the tracebacks on the passed exception objects are incomplete, because part of the traceback is still attached to the ExceptionGroup which you don't have)except*
understands subclassing, butcatch()
doesn't, i.e.,catch({Exception: foo})
will not catch a ValueErrorI think you either need to make
catch()
a total drop-in replacement forexcept*
, whose semantics can be described totally accurately by reference to the 3.11+except*
semantics plus a mechanical syntactic transformation; or else the semantics ofcatch()
need to be documented in much more detail, so that the above differences are obvious rather than having to be determined through experimentation.Your PR is deprecating all of Trio's existing facilities for doing this stuff. The only supported way to catch part of a multi-error at all after your change is to use the
exceptiongroup.catch()
backport (at least until 3.11 comes out in several more months). I'm sorry to be picky about this but I think if we're only giving our users one option it really needs to be rock-solid and extremely clear, and I don't think its current state lives up to that standard.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.
I have updated the implementation of
exceptiongroup.catch()
to match that ofexcept*
and expanded the documentation on its exact semantics in 1.0.0rc6. The idea was indeed that it should be a drop-in replacement (to the extend that the syntax allows, obviously).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.
Can this section be updated for ExceptionGroups instead of being removed? I know it's not really "our" feature anymore, but the existing docs on ExceptionGroups are pretty scant since they're not in a released Python, and we're adopting them as part of our API so it would be nice not to lose friendliness in the transition.
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 there something wrong with the documentation I just added above which specifically shows how to use both
except*
andcatch()
?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's nothing wrong with it, but it does feel like a downgrade when the previous set of docs had multiple examples, describing more edge cases, exception chaining, output, etc.
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.
Ok, I'll restore and update the docs.
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.
I took the time to evaluate the examples from the removed section, but since the catching API in the backport works in such a different way, I'm not sure what there is to document. The backport doesn't provide any functionality that the
except*
syntax doesn't. There's no exception conversion, and to reraise exceptions, you just useraise
as usual. That said, I've amended the documentation to at least show how to iterate over the exceptions in the group.