typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

[question] Proper way to implement `ExceptionGroup.derive` method with the existing overloads?

Open kkirsche opened this issue 2 years ago • 8 comments

Good afternoon,

I was working to implement some custom exception groups in an application that I work on and have had a really hard time getting the types for the derive function to work correctly. Based on reading the documentation linked below, derive is a required method when overriding an exception group to return an instance of it's own class.

Reference

https://docs.python.org/3/library/exceptions.html#BaseExceptionGroup.derive

Reproduction / Example

To reproduce this error, I'll use the example shows in the BaseExceptionGroup page, but with type hints:

from __future__ import annotations

from collections.abc import Sequence
from typing import TypeVar

_ExceptionT_co = TypeVar("_ExceptionT_co", bound=Exception, covariant=True)
_ExceptionT = TypeVar("_ExceptionT", bound=Exception)


class MyGroup(ExceptionGroup[_ExceptionT_co]):
    def derive(self, excs: Sequence[_ExceptionT]) -> MyGroup[_ExceptionT]:
        return MyGroup(self.message, excs)

Mypy output:

❯ mypy ~/Desktop/example.py
/Users/kkirsche/Desktop/example.py:11: error: Signature of "derive" incompatible with supertype "BaseExceptionGroup"  [override]
/Users/kkirsche/Desktop/example.py:11: note:      Superclass:
/Users/kkirsche/Desktop/example.py:11: note:          @overload
/Users/kkirsche/Desktop/example.py:11: note:          def [_ExceptionT <: Exception] derive(self, Sequence[_ExceptionT], /) -> ExceptionGroup[_ExceptionT]
/Users/kkirsche/Desktop/example.py:11: note:          @overload
/Users/kkirsche/Desktop/example.py:11: note:          def [_BaseExceptionT <: BaseException] derive(self, Sequence[_BaseExceptionT], /) -> BaseExceptionGroup[_BaseExceptionT]
/Users/kkirsche/Desktop/example.py:11: note:      Subclass:
/Users/kkirsche/Desktop/example.py:11: note:          def [_ExceptionT <: Exception] derive(self, excs: Sequence[_ExceptionT]) -> MyGroup[_ExceptionT]
Found 1 error in 1 file (checked 1 source file)

Given that this is working off an exception group rather than a base exception group, I'm unclear on what the proper way to fix this would be and was wondering if the team here was able to provide an example that satisfies mypy / typeshed's overrides for BaseExceptionGroup's derive function.

Thoughts

Would it potentially make sense to add a derive method to ExceptionGroup which returns Self rather than a hardcoded ExceptionGroup? This seems to align with how the documentation describes using this behavior, but I'm not confident I understand fully to submit a pull request with recommended change(s).

Thanks for your time and help, hope you all are having a great day.

kkirsche avatar Mar 22 '23 17:03 kkirsche

Hi Kevin, hope you're well!

I'm paging @sobolevn on this one, who authored our current stack of overloads and test cases for ExceptionGroup, and I think understands this stuff much better than I do 😄

AlexWaygood avatar Mar 22 '23 21:03 AlexWaygood

I am sorry, my example was not correct. You can not have exception types there. Here are correct ones:

>>> BaseExceptionGroup('a', [SystemExit()]).derive([ValueError()])
ExceptionGroup('a', [ValueError()])

>>> ExceptionGroup('a', [ValueError()]).derive([SystemExit()])
BaseExceptionGroup('a', [SystemExit()])

This is why we have two overloads. And Self is not actually correct.

sobolevn avatar Mar 26 '23 09:03 sobolevn

But isn't that specifically the base classes not what derive is meant to do when implemented by the user, as outlined in the linked example?

A subclass needs to override it in order to make subgroup() and split() return instances of the subclass rather than ExceptionGroup.

https://docs.python.org/3/library/exceptions.html#BaseExceptionGroup.derive

Just trying to understand how the base classes returning the exception group should be overloaded by the end user given the base exception group overload, specifically.

kkirsche avatar Mar 26 '23 10:03 kkirsche

Yes, this is just the base classes. Custom types can return whatever they like. Do you have any suggestions on how to fix this?

sobolevn avatar Mar 26 '23 11:03 sobolevn

Yes, this is just the base classes. Custom types can return whatever they like.

Do you have any suggestions on how to fix this?

I'm sorry, I was still trying to understand fully what the situation was. Based on your feedback, my understanding is that the example cannot currently be typed given the current overloads, as the separation of BaseException from Exception requires BaseExceptions to return BaseExceptionGroup which isn't a requirement of a custom exception group. Is that a correct understanding of your feedback?

Happy to help brainstorm solutions, I want to make sure I understand how it's supposed to work in a custom error group to understand it if there is an opportunity to change type hints or if instead it's more of a documentation change where the example should call out that handling BaseExceptions should be / may be necessary separate of the Exceptions (at least for type uses).

Thanks for your patience

kkirsche avatar Mar 26 '23 16:03 kkirsche

as the separation of BaseException from Exception requires BaseExceptions to return BaseExceptionGroup which isn't a requirement of a custom exception group. Is that a correct understanding of your feedback?

Yes.

Happy to help brainstorm solutions, I want to make sure I understand how it's supposed to work in a custom error group

I can think of several solutions.

  1. Simplify current annotations to return Self and ignore this corner case. How bad it can be for our users? I don't really think that users will use .derive to pass completely unrelated BaseException instances
  2. Require users to have # type: ignore comment on the overriden methods because this is how implementation works. Basically, users can return instance of their own class for any input

sobolevn avatar Mar 27 '23 08:03 sobolevn

I think I lean towards option 1 of those two solutions, if for no other reason than it aligns with the example use case outlined in the documentation.

An alternative I was wondering about is whether it would make sense to return a Protocol representing an ExceptionGroup or a BaseExceptionGroup instead of a type. That may be completely off base, please let me know if it is, as protocols aren't an area I'm very familiar with in Python.

Looking at the types of ExceptionGroup and BaseExceptionGroup, they seem to be largely compatible:

I haven't attempted to implement it yet, so there may be a gotcha I'm not aware of, but was wondering if this would make sense or not. What is your opinion on this idea?

kkirsche avatar Mar 28 '23 14:03 kkirsche

I ran into this issue as well when writing a custom ExceptionGroup class. The overloads are implementation details, but once we have a workaround, it would be great to document the proper use of type annotations of the example use case.

PIG208 avatar Aug 04 '23 18:08 PIG208