Skip to content
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.

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

functools._lru_cache_wrapper should be a descriptor class, not a callable #6347

Open
erictraut opened this issue Nov 20, 2021 · 10 comments
Open

Comments

@erictraut
Copy link
Contributor

I'm not entirely sure this is a bug in the type stub. It depends on the interpretation of ParamSpec when used with methods.

This is related to the discussion here and this bug report filed in the pyright issue tracker.

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 20, 2021

Could this be a use case for Concatenate?

Leave the stub for _lru_cache_wrapper as it is:

from typing import Generic, TypeVar, ParamSpec, Callable

_P = ParamSpec("_P")
_T = TypeVar("_T")

class _lru_cache_wrapper(Generic[_P, _T]):  # type: ignore
    __wrapped__: Callable[_P, _T]  # type: ignore
    def __call__(self, *args: _P.args, **kwargs: _P.kwargs) -> _T: ...  # type: ignore

But then change the stub for lru_cache/cache to the following:

import sys
from typing import overload, Concatenate, Any

if sys.version_info >= (3, 8):
    @overload
    def lru_cache(maxsize: int | None = ..., typed: bool = ...) -> Callable[[Callable[Concatenate[Any, _P], _T]], _lru_cache_wrapper[_P, _T]]: ...  # type: ignore
    @overload
    def lru_cache(maxsize: Callable[Concatenate[Any, _P], _T], typed: bool = ...) -> _lru_cache_wrapper[_P, _T]: ...  # type: ignore

else:
    def lru_cache(maxsize: int | None = ..., typed: bool = ...) -> Callable[[Callable[Concatenate[Any, _P], _T]], _lru_cache_wrapper[_P, _T]]: ...  # type: ignore

if sys.version_info >= (3, 9):
    def cache(__user_function: Callable[Concatenate[Any, _P], _T]) -> _lru_cache_wrapper[_P, _T]: ...  # type: ignore

@erictraut
Copy link
Contributor Author

I think cache and lru_cache can be used with either methods or non-method functions, right? Wouldn't that approach break for non-methods?

@AlexWaygood
Copy link
Member

Wouldn't that approach break for non-methods?

Good point! Are you able to flesh out the rewrite-it-as-a-descriptor proposal a little?

@erictraut
Copy link
Contributor Author

I was hoping that someone who is more familiar with how these calls are meant to work could suggest a solution.

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 20, 2021

How about this?

import sys
from typing import Generic, TypeVar, ParamSpec, Callable, Concatenate, overload, Type

_P = ParamSpec("_P")
_T = TypeVar("_T")
_S = TypeVar("_S")

class _lru_cache_wrapper(Generic[_S, _P, _T]):  # type: ignore
    __wrapped__: Callable[Concatenate[_S, _P], _T]  # type: ignore
    def __call__(self, __obj: _S, *args: _P.args, **kwargs: _P.kwargs) -> _T: ...  # type: ignore
    def __get__(self, __obj: _S, __type: Type[_S] | None = ...) -> Callable[_P, _T]: ...

if sys.version_info >= (3, 8):
    @overload
    def lru_cache(maxsize: int | None = ..., typed: bool = ...) -> Callable[[Callable[Concatenate[_S, _P], _T]], _lru_cache_wrapper[_S, _P, _T]]: ...  # type: ignore
    @overload
    def lru_cache(maxsize: Callable[Concatenate[_S, _P], _T], typed: bool = ...) -> _lru_cache_wrapper[_S, _P, _T]: ...  # type: ignore

else:
    def lru_cache(maxsize: int | None = ..., typed: bool = ...) -> Callable[[Callable[Concatenate[_S, _P], _T]], _lru_cache_wrapper[_S, _P, _T]]: ...  # type: ignore

if sys.version_info >= (3, 9):
    def cache(__user_function: Callable[Concatenate[_S, _P], _T]) -> _lru_cache_wrapper[_S, _P, _T]: ...  # type: ignore

@erictraut
Copy link
Contributor Author

I don't think that works. This doesn't work with non-method functions that take no parameters.

@cache
def func() -> int:
    return 0

Maybe this isn't the right approach?

Perhaps someone who contributed to PEP 612 could weigh in? @mrkmndz

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 22, 2021

The more we discuss this, the more I agree that this might not be something that can be solved in the stubs (and yes, I'm obviously out of my depth here, so someone more qualified should weigh in). But, well, here's a third attempt at typing lru_cache/cache:

import sys
from typing import Generic, TypeVar, ParamSpec, Callable, Concatenate, overload, NoReturn, Protocol

_P1 = ParamSpec("_P1")
_P2 = ParamSpec("_P2")
_R = TypeVar("_R")
_S = TypeVar("_S")

class _WrapperReturnBase(Protocol[_P1, _R]):
    __wrapped__: Callable[_P1, _R]
    def__call__(self, *args: _P1.args, **kwargs: _P1.kwargs) -> _R: ...
    def cache_info(self) -> _CacheInfo : ...
    def cache_clear(self) -> None: ...

class _WrapperReturnNoArgs(_WrapperReturnBase[[], _R], Protocol[_R]):
    def __get__(self, __obj: _S, __type: Type[_S] | None = ...)  -> Callable[..., NoReturn]: ...

class _WrapperReturnOneArg(_WrapperReturnBase[[_S], _R], Protocol[_S, _R]):
    def __get__(self, __obj: _S, __type: Type[_S] | None = ...) -> _WrapperReturnNoArgs[_R]: ...

class _WrapperReturnMultipleArgs(_WrapperReturnBase[Concatenate[_S, _P2], _R], Protocol[_S, _P2, _R]):
    def __get__(self, __obj: _S, __type: Type[_S] | None = ...) -> _WrapperReturnMultipleArgs[_S, _P2, _R] | _WrapperReturnOneArg[_S, _R]: ...

class _lru_cache_wrapper(Generic[_S, _P, _R]):
    @overload
    def __call__(self, __func: Callable[[], _R]) -> WrapperReturnNoArgs[_R]: ...
    @overload
    def __call__(self, __func: Callable[[_S], _R]) -> _WrapperReturnOneArg[_S, _R]: ...
    @overload
    def __call__(self, __func: Callable[Concatenate[_S, _P2], _R]) -> _WrapperReturnMultipleArgs[_S, _P2, _R]: ...

if sys.version_info >= (3, 8):
    @overload
    def lru_cache(maxsize: int | None = ..., typed: bool = ...) -> _lru_cache_wrapper[_S, _P, _R]: ...
    @overload
    def lru_cache(maxsize: Callable[[], _R], typed: bool = ...) -> _WrapperReturnNoArgs[_R]: ...
    @overload
    def lru_cache(maxsize: Callable[[_S], _R], typed: bool = ...) -> _WrapperReturnOneArg[_S, _R]: ...
    @overload
    def lru_cache(maxsize: Callable[Concatenate[_S, _P2], _R], typed: bool = ...) -> _WrapperReturnMultipleArgs[_S, _P2, _R]: ...

else:
    def lru_cache(maxsize: int | None = ..., typed: bool = ...) -> _lru_cache_wrapper[_S, _P, _R]: ...

if sys.version_info >= (3, 9):
    @overload
    def cache(__user_function: Callable[[], _R]) -> _WrapperReturnNoArgs[_R]: ...
    @overload
    def cache(__user_function: Callable[[_S], _R]) ->  _WrapperReturnOneArg[_S, _R]: ...
    @overload
    def cache(__user_function: Callable[Concatenate[_S, _P2], _R]) -> _WrapperReturnMultipleArgs[_S, _P2, _R]: ...

JukkaL added a commit to JukkaL/typeshed that referenced this issue Nov 22, 2021
This reverts commit 8bda66a.

The change causes issues with ParamSpec implementations in type
checkers, at least pyright and my work-in-progress support for
ParamSpec in mypy. It's not yet clear how to fix the issues, so I
think that it's best to revert this, at least temporarily until we've
found a good solution. See python#6347 for context.
@JukkaL
Copy link
Contributor

JukkaL commented Nov 22, 2021

I also encountered this issue when adding ParamSpec support to mypy. I'm proposing to revert (#6356) the original change until we have a solution that works with all ParamSpec implementations or we at least have clarity about how PEP 612 should be interpreted here.

srittau pushed a commit that referenced this issue Nov 22, 2021
…6356)

This reverts commit 8bda66a.

The change causes issues with ParamSpec implementations in type
checkers, at least pyright and my work-in-progress support for
ParamSpec in mypy. It's not yet clear how to fix the issues, so I
think that it's best to revert this, at least temporarily until we've
found a good solution. See #6347 for context.
JukkaL added a commit to python/mypy that referenced this issue Nov 23, 2021
Add support for type checking several ParamSpec use cases (PEP 612).

@hauntsaninja previously added support for semantic analysis of ParamSpec
definitions, and this builds on top that foundation.

The implementation has these main things going on:
 * `ParamSpecType` that is similar to `TypeVarType` but has three "flavors" that
   correspond to `P`, `P.args` and `P.kwargs`
 * `CallableType` represents `Callable[P, T]` if the arguments are 
   (`*args: P.args`, `**kwargs: P.kwargs`) -- and more generally, there can also
   be arbitrary additional prefix arguments
 * Type variables of functions and classes can now be represented using
   `ParamSpecType` in addition to `TypeVarType`

There are still a bunch of TODOs. Some of these are important to address before the
release that includes this. I believe that this is good enough to merge and remaining 
issues can be fixed in follow-up PRs.

Notable missing features include these:
 * `Concatenate`
 * Specifying the value of ParamSpec explicitly (e.g. `Z[[int, str, bool]]`)
 * Various validity checks -- currently only some errors are caught
 * Special case of decorating a method (python/typeshed#6347)
 * `atexit.register(lambda: ...)` generates an error
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this issue Jan 20, 2022
Add support for type checking several ParamSpec use cases (PEP 612).

@hauntsaninja previously added support for semantic analysis of ParamSpec
definitions, and this builds on top that foundation.

The implementation has these main things going on:
 * `ParamSpecType` that is similar to `TypeVarType` but has three "flavors" that
   correspond to `P`, `P.args` and `P.kwargs`
 * `CallableType` represents `Callable[P, T]` if the arguments are 
   (`*args: P.args`, `**kwargs: P.kwargs`) -- and more generally, there can also
   be arbitrary additional prefix arguments
 * Type variables of functions and classes can now be represented using
   `ParamSpecType` in addition to `TypeVarType`

There are still a bunch of TODOs. Some of these are important to address before the
release that includes this. I believe that this is good enough to merge and remaining 
issues can be fixed in follow-up PRs.

Notable missing features include these:
 * `Concatenate`
 * Specifying the value of ParamSpec explicitly (e.g. `Z[[int, str, bool]]`)
 * Various validity checks -- currently only some errors are caught
 * Special case of decorating a method (python/typeshed#6347)
 * `atexit.register(lambda: ...)` generates an error
@tmke8
Copy link
Contributor

tmke8 commented Mar 4, 2023

I came up with a proposal that aims to obviate the need for using descriptor classes for this, by providing a new mechanism to declare function types with attributes that will then be bound as methods by the usual mechanism if they are defined inside a class: https://mail.python.org/archives/list/[email protected]/thread/35FTOYUG2IPCRIIH3MQKEVV7XW3V7ASB/

@tamird
Copy link
Contributor

tamird commented Feb 23, 2024

What's the latest on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants