Updated type annotations to match those in typeshed #40
Conversation
agronholm
force-pushed
the
update-annotations
branch
from
995b91b
to
5d0e783
Compare
Pull Request Test Coverage Report for Build 3456826194
💛 - Coveralls |
|
@Zac-HD do you have interest in reviewing this? |
| ) -> tuple[BaseExceptionGroup[_BaseExceptionT] | None, Self | None] | tuple[ | ||
| Self | None, Self | None |
There was a problem hiding this comment.
I'm not seeing how we get a non-Self case from this, since we're using .derive()?
There was a problem hiding this comment.
I'm not sure; maybe my implementation misses a case? This is how it was annotated in the typeshed though, so 🤷
There was a problem hiding this comment.
Hmm, might be a typeshed bug then? I'd be fine with merging it as-is though; it doesn't seem particularly important to investigate.
There was a problem hiding this comment.
Ah, got it: the correct return-type annotation is tuple[BaseExceptionGroup[_BaseExceptionT] | None, BaseExceptionGroup[_BaseExceptionT] | None], no use of Self or overrides. That's because the more precise annotation is only true for subclasses which implement .derive(), in which case they should also ship more precise (but still not Self) annotations for .subgroup() and .split().
I'll open an upstream issue against typeshed shortly.
| def __new__(cls, __message: str, __exceptions: Sequence[_ExceptionT_co]) -> Self: | ||
| instance: ExceptionGroup[_ExceptionT_co] = super().__new__( | ||
| cls, __message, __exceptions | ||
| ) | ||
| if cls is ExceptionGroup: |
There was a problem hiding this comment.
Should this be if issubclass(cls, Exception) (or ExceptionGroup)? As-is I could subclass ExceptionGroup and then store a BaseException in that just fine.
There was a problem hiding this comment.
Yes, this is how the upstream implementation works too. Try running this on Python 3.11:
class CustomExcGroup(ExceptionGroup):
pass
exc = CustomExcGroup("message", [SystemExit(), ValueError()])
Works fine. This is somewhat covered in PEP 654: https://peps.python.org/pep-0654/#subclassing-exception-groups
There was a problem hiding this comment.
Yep, I should probably go complain ask about this upstream 😅
|
Thanks for the quick response! This was actually blocking an important upcoming AnyIO changeset from passing mypy checks. |
|
Glad I could help! |
No description provided.