Make ExitStack, AbstractContextManager and AsyncAbstractContextManager generic in return type of __exit__#11048
Conversation
This comment has been minimized.
This comment has been minimized.
|
Quite a few mypy primer hits here. This change probably does not make sense until after PEP696 support. |
This comment has been minimized.
This comment has been minimized.
|
See #11422 for the type var generics feature tracker. |
We could consider adding new ones, to test the functionality though
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This is really encouraging, PEP-696 truly shines here. I'll check if there are any other places in typeshed that use I fixed the obvious ones, the other ones are probably fine using the default as a fallback for now (or maybe we should just set them all to |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ExitStack generic in return type of __exit__ExitStack, Abstract generic in return type of __exit__`
ExitStack, Abstract generic in return type of __exit__`ExitStack, AbstractContextManager and AsyncAbstractContextManager generic in return type of __exit__
|
TypeVar defaults are now available in typeshed. |
This comment has been minimized.
This comment has been minimized.
|
One of the mypy_primer hits is irrelevant, since it is the same unrelated error with different types and the other one looks like an improvement to me. There are a couple of caveats to this change however:
|
|
https://discuss.python.org/t/passing-only-one-typevar-of-two-when-using-defaults/49134 clued me into an issue, since at runtime |
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.
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.
|
The new primer hit seems fine to me. I think the only reason that previously didn't error is because |
|
Diff from mypy_primer, showing the effect of this PR on open source code: werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/debug/__init__.py:368: error: Unused "type: ignore" comment [unused-ignore]
mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ pymongo/__init__.py:176: error: Incompatible return value type (got "_TimeoutContext", expected "ContextManager[None]") [return-value]
+ pymongo/__init__.py:176: note: Following member(s) of "_TimeoutContext" have conflicts:
+ pymongo/__init__.py:176: note: Expected:
+ pymongo/__init__.py:176: note: def __enter__(self) -> None
+ pymongo/__init__.py:176: note: Got:
+ pymongo/__init__.py:176: note: def __enter__(self) -> _TimeoutContext
|
srittau
left a comment
There was a problem hiding this comment.
LGTM, the new primer hits are promising. I'll leave it open a bit, in case another maintainer wants to chime in.
See https://discuss.python.org/t/add-an-else-clause-to-with-statements-to-detect-early-termination/38031 for related discussion.