Return types for (Base)ExceptionGroup.derive(), .subgroup(), and .split() should not use Self

[Zac-HD]

In the following stubs, each use of Self in a return type is incorrect:

typeshed/stdlib/builtins.pyi

Lines 1947 to 1959 in c626137

	@overload
	def subgroup(
	self, __condition: type[_BaseExceptionT] | tuple[type[_BaseExceptionT], ...]
	) -> BaseExceptionGroup[_BaseExceptionT] | None: ...
	@overload
	def subgroup(self: Self, __condition: Callable[[_BaseExceptionT_co], bool]) -> Self | None: ...
	@overload
	def split(
	self: Self, __condition: type[_BaseExceptionT] | tuple[type[_BaseExceptionT], ...]
	) -> tuple[BaseExceptionGroup[_BaseExceptionT] | None, Self | None]: ...
	@overload
	def split(self: Self, __condition: Callable[[_BaseExceptionT_co], bool]) -> tuple[Self | None, Self | None]: ...
	def derive(self: Self, __excs: Sequence[_BaseExceptionT_co]) -> Self: ...

This is because .derive() must be manually overridden to return custom subtypes, in which case the child class should also manually update the type annotations on .subgroup() and .split()¹. For example:

class MyGroup(ExceptionGroup):
    pass

excs = [Exception("oops")]
mg = MyGroup("msg", excs)
assert type(mg.derive(excs)) == ExceptionGroup  # *not* the Self type!

Discovered via agronholm/exceptiongroup#40 (comment); if accepted here we should make sure to fix the backport too.

Footnotes

1. since I don't know of a way to express "that other method's return-type" in an annotation ↩

[sobolevn]

Hi @Zac-HD! Glad to see you in typing-land! 👋

Related PRs:

• ExceptionGroup: make it generic #7626
• add (Base)ExceptionGroup #6655

Yes, looks like Self contract is not always respected. Examples:

>>> class MyGroup(ExceptionGroup):
...     pass
>>> mg = MyGroup("msg", [ValueError(), TypeError()])

split

>>> mg.split(ValueError)
(ExceptionGroup('msg', [ValueError()]), ExceptionGroup('msg', [TypeError()]))
>>> mg.split((UnicodeDecodeError, UnicodeEncodeError, TypeError))
(ExceptionGroup('msg', [TypeError()]), ExceptionGroup('msg', [ValueError()]))

>>> mg.split(IndexError)
(None, ExceptionGroup('msg', [ValueError(), TypeError()]))

>>> mg.split(lambda e: True)
(MyGroup('msg', [ValueError(), TypeError()]), None)
>>> mg.split(lambda e: isinstance(e, ValueError))
(ExceptionGroup('msg', [ValueError()]), ExceptionGroup('msg', [TypeError()]))

Self is only preserved when no split happens in the left part (I think it is not really important, we can always use ExceptionGroup).

derive

>>> b = BaseExceptionGroup('m', [ValueError()])
>>> b.derive([TypeError()])  # upcast
ExceptionGroup('m', [TypeError()])  

>>> mg = MyGroup("msg", [ValueError(), TypeError()])
>>> mg.derive([IndexError()])
ExceptionGroup('msg', [IndexError()])
>>> mg.derive([ValueError(), TypeError()])  # exactly the same args
ExceptionGroup('msg', [ValueError(), TypeError()])

I cannot find a case, where Self is preserved.

subgroup

>>> mg.subgroup(lambda e: True)
MyGroup('msg', [ValueError(), TypeError()])

>>> mg.subgroup(lambda e: False)
>>> 

>>> mg.subgroup(TypeError)
ExceptionGroup('msg', [TypeError()])

>>> b = BaseExceptionGroup('m', [ValueError()])
>>> b.subgroup(TypeError)
>>> b.subgroup(ValueError)
ExceptionGroup('m', [ValueError()])

So, it can either be None or ExceptionGroup or MyGroup: so we can go with ExceptionGroup.

I would like to work on it, if no one has objections.

[Zac-HD]

I only mostly stick to runtime evil tricks, good to be here 😁

That all sounds good; just note under the derive examples that when b = BaseExceptionGroup('m', [ValueError()]) b is an instance of ExceptionGroup due to the unusual nature of BaseExceptionGroup.__new__! In fact that should maybe be added as an overload, though it gets tricky thinking through the subclasses...🤔

[sobolevn]

due to the unusual nature of BaseExceptionGroup.new

I will take a look, I've missed this part. Thanks!

[sobolevn]

For the record:

static PyObject *
BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
    struct _Py_exc_state *state = get_exc_state();
    PyTypeObject *PyExc_ExceptionGroup =
        (PyTypeObject*)state->PyExc_ExceptionGroup;

    PyObject *message = NULL;
    PyObject *exceptions = NULL;

    if (!PyArg_ParseTuple(args,
                          "UO:BaseExceptionGroup.__new__",
                          &message,
                          &exceptions)) {
        return NULL;
    }

    if (!PySequence_Check(exceptions)) {
        PyErr_SetString(
            PyExc_TypeError,
            "second argument (exceptions) must be a sequence");
        return NULL;
    }

    exceptions = PySequence_Tuple(exceptions);
    if (!exceptions) {
        return NULL;
    }

    /* We are now holding a ref to the exceptions tuple */

    Py_ssize_t numexcs = PyTuple_GET_SIZE(exceptions);
    if (numexcs == 0) {
        PyErr_SetString(
            PyExc_ValueError,
            "second argument (exceptions) must be a non-empty sequence");
        goto error;
    }

    bool nested_base_exceptions = false;
    for (Py_ssize_t i = 0; i < numexcs; i++) {
        PyObject *exc = PyTuple_GET_ITEM(exceptions, i);
        if (!exc) {
            goto error;
        }
        if (!PyExceptionInstance_Check(exc)) {
            PyErr_Format(
                PyExc_ValueError,
                "Item %d of second argument (exceptions) is not an exception",
                i);
            goto error;
        }
        int is_nonbase_exception = PyObject_IsInstance(exc, PyExc_Exception);
        if (is_nonbase_exception < 0) {
            goto error;
        }
        else if (is_nonbase_exception == 0) {
            nested_base_exceptions = true;
        }
    }

    PyTypeObject *cls = type;
    if (cls == PyExc_ExceptionGroup) {
        if (nested_base_exceptions) {
            PyErr_SetString(PyExc_TypeError,
                "Cannot nest BaseExceptions in an ExceptionGroup");
            goto error;
        }
    }
    else if (cls == (PyTypeObject*)PyExc_BaseExceptionGroup) {
        if (!nested_base_exceptions) {
            /* All nested exceptions are Exception subclasses,
             * wrap them in an ExceptionGroup
             */
            cls = PyExc_ExceptionGroup;
        }
    }
    else {
        /* Do nothing - we don't interfere with subclasses */
    }

    if (!cls) {
        /* Don't crash during interpreter shutdown
         * (PyExc_ExceptionGroup may have been cleared)
         */
        cls = (PyTypeObject*)PyExc_BaseExceptionGroup;
    }
    PyBaseExceptionGroupObject *self =
        _PyBaseExceptionGroupObject_cast(BaseException_new(cls, args, kwds));
    if (!self) {
        goto error;
    }

    self->msg = Py_NewRef(message);
    self->excs = exceptions;
    return (PyObject*)self;
error:
    Py_DECREF(exceptions);
    return NULL;
}

[sobolevn]

Looks like I found a bug in ExceptionGroup.subgroup:

>>> b = BaseExceptionGroup('m', [TypeError(), OSError(1, 2)])
>>> def p(e):
...    print(type(e), e)
...    return True
... 
>>> b.subgroup(p)
<class 'ExceptionGroup'> m (2 sub-exceptions)
ExceptionGroup('m', [TypeError(), PermissionError(1, 2)])

Notice that p is called:

1. once, not twice (for TypeError and OSError)
2. with ExceptionGroup instance

While the docs say:

   .. method:: subgroup(condition)

      Returns an exception group that contains only the exceptions from the
      current group that match *condition*, or ``None`` if the result is empty.

      The condition can be either a function that accepts an exception and returns
      true for those that should be in the subgroup, or it can be an exception type
      or a tuple of exception types, which is used to check for a match using the
      same check that is used in an ``except`` clause.

[sobolevn]

The same happens with .split:

>>> b.split(p)
<class 'ExceptionGroup'> m (2 sub-exceptions)
(ExceptionGroup('m', [TypeError(), PermissionError(1, 2)]), None)

[sobolevn]

Ok, this is quite tricky. This function is firstly called with ExceptionGroup itself.
Then it is called with exceptions only if the first call returned False. If the first call returns True, it is not called anymore.

[AlexWaygood]

Has this been resolved by #9230, or is there more still to do?

[sobolevn]

I think so! (I cannot close issues, please send help 😆 )

Thanks, @Zac-HD, for the report!

[AlexWaygood]

Yes, thanks @Zac-HD for the report and thanks @sobolevn for the fix! If there's any further problems, please feel free to report them!

[Zac-HD]

Happy to help, and I will!