Add _typeshed.(Opt)ExcInfo #7645
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| from types import FrameType, TracebackType | ||
| from typing import IO, Any, Callable | ||
| from typing_extensions import TypeAlias | ||
|
|
||
| _ExcInfo: TypeAlias = tuple[type[BaseException] | None, BaseException | None, TracebackType | None] | ||
|
|
There was a problem hiding this comment.
Hmm, tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None] is a different type to tuple[type[BaseException] | None, BaseException | None, TracebackType | None], so this is a semantic change
There was a problem hiding this comment.
Indeed, but usually the latter was a shortcut used for the former. See the typical __exit__ annotation that should properly be annotated using an overload.
There was a problem hiding this comment.
Right, the latter is a supertype of the former
There was a problem hiding this comment.
I believe it's not strictly true that it's always either all-None or all-non-None, at least before 3.11. (Irit did some work in 3.11 that made it always true.) See here: https://github.com/python/cpython/blob/84c279b514141f608cf480905c87d48998e296d1/Python/ceval.c#L4474
There was a problem hiding this comment.
This could be a remnant of Python 2 string exceptions.
There was a problem hiding this comment.
Maybe it's better to err on the side of safety here and define _typeshed.OptExcInfo as tuple[type[BaseException] | None, BaseException | None, TracebackType | None] instead of tuple[type[BaseException], BaseException, TracebackType] | tuple[None, None, None]
There was a problem hiding this comment.
Primer output looks good and from a type safety perspective, I'd rather go with the more strict/correct annotation. Personally, I've not come across a situation where the type is None when the other fields are not, and allowing that could certainly introduce subtle bugs. It would also disagree with the documentation of both sys.exc_info and contextmanager.__exit__.
There was a problem hiding this comment.
@JelleZijlstra, do you know of any real-world examples where some (but not all) elements of the tuple are None?
If not, I'm fine with merging this, as it's probably just a theoretical issue :)
There was a problem hiding this comment.
From python/cpython#89874 (comment), sounds like it could only happen with interesting use of the C API, so probably not worth worrying about.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
No description provided.