make contextmanager return AbstractContextManager #6521
Conversation
Fixes #6520. There might be a good reason to keep the current implementation, but I want to see what CI says. I believe mypy has a plugin for `@contextmanager`, so it may not show that much. I found no motivation for the separate class in https://github.com/python/typeshed/pulls?q=is%3Aissue+GeneratorContextManager+.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
| @@ -36,7 +36,7 @@ class _GeneratorContextManager(AbstractContextManager[_T_co]): | |||
| def __call__(self, func: _F) -> _F: ... | |||
|
|
|||
| # type ignore to deal with incomplete ParamSpec support in mypy | |||
| def contextmanager(func: Callable[_P, Iterator[_T]]) -> Callable[_P, _GeneratorContextManager[_T]]: ... # type: ignore[misc] | |||
There was a problem hiding this comment.
_GeneratorContextManager is also used in stubs/decorator/decorator.pyi.
- Should we change
decorator.pyitoo? - If we change both, should we delete
_GeneratorContextManager?
There was a problem hiding this comment.
I don't know about decorator; if it uses the class we can keep it.
There was a problem hiding this comment.
Apparently decorator's documentation explains what the __call__ method is supposed to do: https://github.com/micheles/decorator/blob/master/docs/documentation.md#contextmanager
Maybe we should just keep it, because it is a documented feature.
There was a problem hiding this comment.
Another observation: the implementation of decorator just does from contextlib import _GeneratorContextManager, and uses it as a base class. So their solution to #6520 (or rather, a similar problem at runtime) is to just import it and not worry about it.
|
The separate class supports a documented behavior of |
Fixes #6520.
There might be a good reason to keep the current implementation, but I want to see what CI says. I believe mypy has a plugin for
@contextmanager, so it may not show that much.I found no motivation for the separate class in https://github.com/python/typeshed/pulls?q=is%3Aissue+GeneratorContextManager+.