Skip to content

Support non-sequence iterables in the ExceptionGroup API, or document the design decision #129867

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.

Sign up for GitHub

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

Jump to bottom
Open
bswck opened this issue Feb 8, 2025 · 4 comments
Open

Support non-sequence iterables in the ExceptionGroup API, or document the design decision #129867

bswck opened this issue Feb 8, 2025 · 4 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@bswck
Copy link
Contributor

bswck commented Feb 8, 2025 •
edited
Loading

Feature or enhancement

Proposal:

Currently, the ExceptionGroup API only supports sequences of exceptions to its second parameter excs.
This ticket suggests either:

  • additively changing the API to support non-sequence iterables as inputs as well.

or

  • documenting why only sequences are supported.

Currently, this Python code would work:

ExceptionGroup("list", [ValueError(), AttributeError()])  # OK

ExceptionGroup("tuple", (ValueError(), AttributeError()))  # OK

class Niche:
    def __getitem__(self, i):
        if i == 0:
            return ValueError()
        if i == 1:
            return AttributeError()
        raise StopIteration

ExceptionGroup("niche", Niche())  # OK

Note

AFAIK, ExceptionGroup objects do not store the sequence passed to them, unless it's a tuple object (excluding tuple subclasses). They export a new tuple (created from the passed-in sequence) or passed-in tuple via .exceptions.
Quick illustration:

assert ExceptionGroup("", excs := [ValueError()]).exceptions is not excs  # OK
assert ExceptionGroup("", excs := (ValueError(),)).exceptions is excs  # OK
assert ExceptionGroup("", excs := type("", (tuple,), {})([ValueError(),])).exceptions is not excs  # OK

This is a minimal example of a use case I would desire, which is not currently supported:

def cb1():
    raise ValueError

def cb2():
    raise AttributeError

errors = {}
callbacks = {"module1": cb1, "module2": cb2}
for mod, cb in callbacks.items():
    try:
        cb()
    except Exception as exc:
        errors[mod] = exc

if errors:
    raise ExceptionGroup(f"got errors in {', '.join(errors)}", errors.values())

Important

While this can be simply worked around by converting the values' view into a sequence, the question is more fundamental: why?

There are some other variants of this on GitHub, too (including my humble one).

It was an intentional decision to only support sequences, justifying the very first part of the constructor test:

cpython/Lib/test/test_exception_group.py

Lines 39 to 42 in 1bccd6c

def test_bad_EG_construction__bad_excs_sequence(self):
MSG = r'second argument \(exceptions\) must be a sequence'
with self.assertRaisesRegex(TypeError, MSG):
ExceptionGroup('errors not sequence', {ValueError(42)})

Before writing this issue, I did a research in some places that seemed like they could explain why only sequences made a cut. Namely, I:

I didn't have the time to read them very in depth, so I might have overlooked something; I only found this thread relevant:

Which leads me to think that supporting Iterable was initially planned (the author meant Iterable[Exception] instead of Iterable[ExceptionGroup]), but then implicitly ignored for the sake of predicted use cases only being with sequences (lists/tuples).
Please note that the considerations about variadic constructor of BaseExceptionGroup are not relevant to this ticket.

Before writing this, I also reached out to @ZeroIntensity and @Eclips4 to discuss the topic.

@Eclips4 confirmed that he saw no hard requirement for the exception sequence to be a sequence specifically, and applying this simple patch

diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index 154cde93168..f97eb33953f 100644
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@@ -871,7 +871,8 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         return NULL;
     }

-    if (!PySequence_Check(exceptions)) {
+    PyTypeObject *t = Py_TYPE(exceptions);
+    if (t->tp_iter == NULL) {
         PyErr_SetString(
             PyExc_TypeError,
             "second argument (exceptions) must be a sequence");

only causes test_bad_EG_construction__bad_excs_sequence to fail.

CC @iritkatriel @gvanrossum @1st1

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response
@bswck bswck added the type-feature A feature request or enhancement label Feb 8, 2025
@picnixz picnixz added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 8, 2025
@iritkatriel
Copy link
Member

Is this a theoretical question or is there a use case for non-sequence iterable?

PEP 654 specifies sequence, so if there is a compelling use case I believe it would require a new PEP or an amendment of this PEP.

@bswck
Copy link
Contributor Author

bswck commented Feb 8, 2025 •
edited
Loading

Hi @iritkatriel, I included the use case in this issue.

@bswck
Copy link
Contributor Author

bswck commented Feb 8, 2025 •
edited
Loading

I think there can be more of them. Generally, IMO:
a) There is no compelling reason to not support all iterables. Or is there?
b) It was initially planned to support all iterables, without a clear track why the initial plan changed later on.
c) There is at least one use case, and there can be others, too.
d) Supporting all iterables offers more flexibility.
e) The cost of supporting all iterables is a very simple patch, updating the docs and the typeshed, and it is as well an opportunity to remove one assertion in a test entirely (replacing it with a more general test, perhaps).

This is an additive change compatible with the PEP (I see it would be necessary to amend it, yes). It's not breaking unless someone bases their workflow on

try:
    ExceptionGroup("", {ValueError()})
except TypeError:
    main_logic()

@iritkatriel
Copy link
Member

The reality is that we have a process that we need to follow when we make changes to builtins, and this includes specifying the change in a PEP and putting the proposal under quite a bit of scrutiny. Had you pointed to a discrepancy between the implementation and the PEP, we would have fixed it as a bug. Since the implementation matches the PEP in this case, this is a feature request involving a change to a builtin type, and the burden of motivating the change there. This feature would require a language spec change (impacting other python implementations too). One thing we would need to decide is whether the language spec should require that all iterables are supported, or merely allow it.

I noticed the code snippet you posted, but it looked more like "here is some code that would work if you add the feature", rather than an actual problem that came up in real life programming. Sorry I jumped to that conclusion, but now I'm curious -is someone really building a dict of exceptions and then creating an exception group, and then ... doing what with the dict?

@ZeroIntensity ZeroIntensity mentioned this issue Mar 1, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@iritkatriel @picnixz @bswck