Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

bpo-46729: improved str() for BaseExceptionGroup#31294

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
iritkatriel merged 4 commits intopython:mainfromiritkatriel:eg_str
Feb 22, 2022

Conversation

iritkatriel
Copy link
Member

@iritkatrieliritkatriel commentedFeb 12, 2022
edited
Loading

Draft PR to discuss options for a better Exception Group str().

Initial suggestion is to add "(group of X exceptions)", but this seems a bit too long.

https://bugs.python.org/issue46729

@iritkatriel
Copy link
MemberAuthor

@1st1 See#31270 (comment) .

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

It feels weird to report the number of nodes in the tree without any indication of the tree structure. I would be okay if the number just was the count of the immediate subexceptions (i.e.,len(eg.exceptions)). If you want to show the number of leaves: (a) don't count interior nodes!; (b) add something to the message indicating that the tree isn't flat (if it isn't). Maybe make the message "N sub-exceptions" ifN == len(eg.exceptions) but "N sub-exceptions in tree of depth D" if there are sub-sub (etc.) exceptions?

@iritkatriel
Copy link
MemberAuthor

It feels weird to report the number of nodes in the tree without any indication of the tree structure. I would be okay if the number just was the count of the immediate subexceptions (i.e.,len(eg.exceptions)). If you want to show the number of leaves: (a) don't count interior nodes!; (b) add something to the message indicating that the tree isn't flat (if it isn't). Maybe make the message "N sub-exceptions" ifN == len(eg.exceptions) but "N sub-exceptions in tree of depth D" if there are sub-sub (etc.) exceptions?

I'd go with just the number of directly contained exceptions, because a nested group is really just one error from some group operation.

But I wonder if any count is really useful information here. We said iteration is irrelevant because it's not about how many but about what types of exceptions are included. Maybe we should drop the number and indicate in some other way that this is a group of exceptions?

@gvanrossum
Copy link
Member

Yury’s MultiError had the set of exception names in the message (as well as the count). Maybe that’s an idea?

@gvanrossum
Copy link
Member

That could be for the entire tree (leaves only), OR only the first level (so it might show EG in there). If too many, use ‘…’ .

@iritkatriel
Copy link
MemberAuthor

Yury’s MultiError had the set of exception names in the message (as well as the count). Maybe that’s an idea?

Yury had the whole traceback in str(), right?

The str() appears in the standard traceback, so the types would be repetitive.

Also we would want to dedup them, so we would need to construct a set of the types. We don’t want to allocate memory while rendering an exception, so we will need to do this in the constructor and save the set.

@gvanrossum
Copy link
Member

Yury’s MultiError had the set of exception names in the message (as well as the count). Maybe that’s an idea?

Yury had the whole traceback in str(), right?

Yeah, but the first line has this appended the the message argument: "N sub errors: (TYPES)" where N is len(errors) and TYPES is the set of error types -- both only using the immediate child level.
see here.

The str() appears in the standard traceback, so the types would be repetitive.

But the traceback is pretty verbose -- the first line of the message contains a useful summary. (Admittedly mostly used by tests.)

Also we would want to dedup them, so we would need to construct a set of the types. We don’t want to allocate memory while rendering an exception, so we will need to do this in the constructor and save the set.

Or the string -- perhaps more work but a simpler type.

Then again maybe we should just stick with what we have.

@iritkatriel
Copy link
MemberAuthor

On second thought I don’t think it’s a problem to put the number as a kind of summary and indication that it’s a group.

The types appear in the repr() so maybe that’s enough.

In the original implementation str() just printed the msg and then in the traceback we added a line under the msg like “with x sub-exceptions”. We removed that later because we thought it was clutter. It makes more sense to put it on the exception str() for when that’s used in other contexts, and also it doesn’t add a line to the traceback.

@1st1 do you have a view?

@gvanrossum
Copy link
Member

If the str() would just bef"{self.message} ({len(self.exceptions)} sub-exceptions)" that would be nice. Agreed the repr() is good as it is.

@iritkatriel
Copy link
MemberAuthor

I made a draft PR for the PEP update:python/peps#2356
I don't think we ever specified str() and repr() in the PEP, it's just implied from the examples.

Should I now ask the SC to approve it?

@iritkatrieliritkatriel marked this pull request as ready for reviewFebruary 22, 2022 17:19
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@gvanrossumgvanrossumgvanrossum approved these changes

@1st11st1Awaiting requested review from 1st1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@iritkatriel@gvanrossum@the-knights-who-say-ni@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp