Skip to content

Improve [Async]ContextDecorator type hinting #13416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 20, 2025
Merged

Improve [Async]ContextDecorator type hinting #13416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
97ffc0d
Improve [Async]ContextDecorator type hinting
ncoghlan Jan 20, 2025
4b59fc8
Fix typo
ncoghlan Jan 20, 2025
0757c3a
Inherit __call__ signature in decorator stub
ncoghlan Jan 20, 2025
d5d93d3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 20, 2025
d79f697
Remove no longer used TypeVar
ncoghlan Jan 20, 2025
1fb52db
_R typevar replaced the last use of _F in contextlib.pyi
ncoghlan Jan 20, 2025
cebcc7e
Attempt to fix callable signature checks in jax
ncoghlan Jan 20, 2025
d804d44
Callable is a type form, not a type
ncoghlan Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions stdlib/contextlib.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ _T = TypeVar("_T")
_T_co = TypeVar("_T_co", covariant=True)
_T_io = TypeVar("_T_io", bound=IO[str] | None)
_ExitT_co = TypeVar("_ExitT_co", covariant=True, bound=bool | None, default=bool | None)
_F = TypeVar("_F", bound=Callable[..., Any])
_G = TypeVar("_G", bound=Generator[Any, Any, Any] | AsyncGenerator[Any, Any], covariant=True)
_P = ParamSpec("_P")
_R = TypeVar("_R")

_SendT_contra = TypeVar("_SendT_contra", contravariant=True, default=None)
_ReturnT_co = TypeVar("_ReturnT_co", covariant=True, default=None)
Expand Down Expand Up @@ -64,9 +64,13 @@ class AbstractAsyncContextManager(ABC, Protocol[_T_co, _ExitT_co]): # type: ign
self, exc_type: type[BaseException] | None, exc_value: BaseException | None, traceback: TracebackType | None, /
) -> _ExitT_co: ...

class _WrappedCallable(Generic[_P, _R]):
__wrapped__: Callable[_P, _R]
def __call__(self, *args: _P.args, **kwargs: _P.kwargs) -> _R: ...

class ContextDecorator:
def _recreate_cm(self) -> Self: ...
def __call__(self, func: _F) -> _F: ...
def __call__(self, func: Callable[_P, _R]) -> _WrappedCallable[_P, _R]: ...

class _GeneratorContextManagerBase(Generic[_G]):
# Ideally this would use ParamSpec, but that requires (*args, **kwargs), which this isn't. see #6676
Expand All @@ -93,11 +97,11 @@ class _GeneratorContextManager(
def contextmanager(func: Callable[_P, Iterator[_T_co]]) -> Callable[_P, _GeneratorContextManager[_T_co]]: ...

if sys.version_info >= (3, 10):
_AF = TypeVar("_AF", bound=Callable[..., Awaitable[Any]])
_AR = TypeVar("_AR", bound=Awaitable[Any])

class AsyncContextDecorator:
def _recreate_cm(self) -> Self: ...
def __call__(self, func: _AF) -> _AF: ...
def __call__(self, func: Callable[_P, _AR]) -> _WrappedCallable[_P, _AR]: ...

class _AsyncGeneratorContextManager(
_GeneratorContextManagerBase[AsyncGenerator[_T_co, _SendT_contra]],
Expand Down
4 changes: 1 addition & 3 deletions stubs/decorator/decorator.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ from re import Pattern
from typing import Any, Literal, TypeVar
from typing_extensions import ParamSpec

_C = TypeVar("_C", bound=Callable[..., Any])
_Func = TypeVar("_Func", bound=Callable[..., Any])
_T = TypeVar("_T")
_P = ParamSpec("_P")
Expand Down Expand Up @@ -65,8 +64,7 @@ def decorator(
caller: Callable[..., Any], _func: Callable[..., Any] | None = ...
) -> Callable[[Callable[..., Any]], Callable[..., Any]]: ...

class ContextManager(_GeneratorContextManager[_T]):
def __call__(self, func: _C) -> _C: ...
Comment on lines -68 to -69
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove __call__ here? The decorator library explicitly adds it and it's not part of the inheritance tree:

https://github.com/micheles/decorator/blob/519cc713878df4bfb768edfaa65e4bf2d875e421/src/decorator.py#L314-L318

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the stub tests failed complaining that the __call__ definition here no longer conformed to the parent class declaration of ContextDecorator.__call__ (it was the same issue as in jax: the return type isn't the same as the input type, it just has the same call signature).

Deleting the stub in the subclass means it inherits the ContextDecorator.__call__ signature, which is also correct for decorator.ContextManager.

I didn't check if the nominal signature of decorator.decorate itself might need updating, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I edited the PR description to include that additional information about the change fixing a genuine bug in the old signature definition.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, I missed some of the base classes of _GeneratorContextManager when checking (twice!).

class ContextManager(_GeneratorContextManager[_T]): ...

def contextmanager(func: Callable[_P, Iterator[_T]]) -> Callable[_P, ContextManager[_T]]: ...
def append(a: type, vancestors: list[type]) -> None: ...
Expand Down
Loading