Skip to content

Conversation

@dr-carlos
Copy link
Contributor

@dr-carlos dr-carlos commented Dec 7, 2025

@picnixz
Copy link
Member

picnixz commented Dec 7, 2025

Was it discussed in the PR and deemed a better way alternative? I personally prefer using [] because it's more natural to () on my keyboard but I wonder whether we really need to do mention this in the docs. It's fine to use a list right? I also think it has a nicer rendering as otherwise we have nested (.

@dr-carlos
Copy link
Contributor Author

dr-carlos commented Dec 7, 2025

Was it discussed in the PR and deemed a better way alternative? I personally prefer using [] because it's more natural to () on my keyboard but I wonder whether we really need to do mention this in the docs. It's fine to use a list right? I also think it has a nicer rendering as otherwise we have nested (.

It certainly isn't vastly superior, just slightly more efficient. @iritkatriel asked me to open an issue to update the docs, so I opened an issue and PR for it since there aren't many changes.

When you run repr() on an ExceptionGroup, if the original sequence was a list, then we need to create a new list from the eg.exceptions tuple, which could be expensive for a large list. This step is unnecessary for tuples as we just use the repr for eg.exceptions straight-up. So it's slightly more efficient (though of minimal effect for small groups).

@picnixz
Copy link
Member

picnixz commented Dec 7, 2025

Yes it could be expensive but exception handlers are usually rare paths, so performance usually aren't an issue here. Unless you have a list of millions of exception objects, it doesn't really matter IMO.

@iritkatriel
Copy link
Member

I agree it's not a significant perf impact. My point is it's inconsistent to recommend in the docs to use tuples and then have examples using [].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review docs Documentation in the Doc dir skip news

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants