Skip to content
This repository was archived by the owner on Apr 10, 2022. It is now read-only.

edit raise in except* example #13

Merged
merged 4 commits into from
Jan 17, 2021
Merged

edit raise in except* example #13

merged 4 commits into from
Jan 17, 2021

Conversation

iritkatriel
Copy link
Member

No description provided.

@iritkatriel iritkatriel requested review from 1st1 and gvanrossum January 3, 2021 23:15
except_star.md Outdated

# would result in:
#
# ExceptionGroup(
# ExceptionGroup(
# "????", <-- TODO: what is the message of this EG?
Copy link
Member

Choose a reason for hiding this comment

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

Let's use the empty string. Then I believe it will be rendered as

ExceptionGroup

instead of

ExceptionGroup: some message

except_star.md Outdated
@@ -199,14 +204,14 @@ except *OSerror as errors:
for e in errors:
if e.errno != errno.EPIPE:
new_errors.append(e)
raise ExceptionGroup(*new_errors)
raise ExceptionGroup(e.msg, *new_errors)
Copy link
Member

Choose a reason for hiding this comment

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

‘e.msg’ seems wrong here, e is a leftover from the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, should be errors.msg.

@1st1
Copy link
Member

1st1 commented Jan 5, 2021

I'm not sure if the ExceptionGroup.__init__(message: str, *errors: BaseException) signature makes sense from the typing standpoint. I.e. it's common to access the message of the exception object via ex.args[0] and it feels wrong that the type of ex.args[0] will be str, and the type of ex.args[1:] will be Iterable[BaseException].

So perhaps it should be (message: str, errors: List[BaseException]).

@gvanrossum
Copy link
Member

We should do whatever is the most convenient for the caller. The type can be dealt with later (and ex.args is hardly a great API anyway).

But it looks like the most common use case is probably when we have naturally collected the sub-exceptions in a list already, since we catch them one at a time. So then your proposal of using a single list (sequence) argument makes sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants