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

improve lru_cache by using ParamSpec #7771

Closed
wants to merge 10 commits into from
Closed

improve lru_cache by using ParamSpec #7771

wants to merge 10 commits into from

Conversation

asottile
Copy link
Contributor

@asottile asottile commented May 2, 2022

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@asottile
Copy link
Contributor Author

asottile commented May 3, 2022

hmmm, this is close but not quite there. seems my little descriptor trick fixes a lot of situations but doesn't quite work for classmethods. also seems to trigger an internal error in mypy (though pyright is ok with it) -- mypy's matching of Callable with ParamSpec seems a little incomplete

I had originally written this idea as a WIP mypy plugin

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

AlexWaygood commented May 3, 2022

Linking to previous typeshed discussion on this topic:

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

stdlib/functools.pyi Outdated Show resolved Hide resolved
@AlexWaygood
Copy link
Member

I've reported the mypy crash at python/mypy#12922.

@AlexWaygood
Copy link
Member

I've reported the mypy crash at python/mypy#12922.

The crash should be fixed in 0.970 now that python/mypy#12953 has been merged :)

@AlexWaygood
Copy link
Member

Marking this as deferred until mypy 0.970 has been released.

@AlexWaygood AlexWaygood added the status: deferred Issue or PR deferred until some precondition is fixed label Jun 12, 2022
stdlib/functools.pyi Outdated Show resolved Hide resolved
stdlib/functools.pyi Outdated Show resolved Hide resolved
stdlib/functools.pyi Outdated Show resolved Hide resolved
@AlexWaygood AlexWaygood removed the status: deferred Issue or PR deferred until some precondition is fixed label Jul 19, 2022
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

The patch no longer causes mypy to crash on 0.971, but there's now a stubtest failure. Also, the mypy_primer report is a bit of a mixed bag.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@kkirsche

This comment was marked as off-topic.

@AlexWaygood

This comment was marked as resolved.

@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood added the status: needs decision Needs a final decision by the typeshed maintainers label Aug 20, 2022
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

jax (https://github.com/google/jax)
+ jax/experimental/sharding.py:94: error: Signature of "shard_shape" incompatible with supertype "Sharding"  [override]
+ jax/experimental/sharding.py:304: error: Signature of "devices_indices_map" incompatible with supertype "Sharding"  [override]
+ jax/experimental/sharding.py:317: error: Signature of "shard_shape" incompatible with supertype "XLACompatibleSharding"  [override]
+ jax/experimental/sharding.py:317: error: Signature of "shard_shape" incompatible with supertype "Sharding"  [override]
+ jax/experimental/sharding.py:378: error: Signature of "devices_indices_map" incompatible with supertype "Sharding"  [override]

manticore (https://github.com/trailofbits/manticore)
+ manticore/core/smtlib/solver.py:505: error: Signature of "can_be_true" incompatible with supertype "Solver"

pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/dtypes/cast.py:560: error: Unused "type: ignore" comment

pydantic (https://github.com/samuelcolvin/pydantic)
+ pydantic/tools.py:37: error: Unused "type: ignore" comment

jinja (https://github.com/pallets/jinja)
+ src/jinja2/environment.py:1184: error: Unused "type: ignore" comment
+ src/jinja2/environment.py:1199: error: Unused "type: ignore" comment

ibis (https://github.com/ibis-project/ibis)
+ ibis/backends/duckdb/tests/conftest.py:19: error: Too many arguments for "__call__" of "_lru_cache_wrapper_0"
+ ibis/backends/duckdb/tests/conftest.py:53: error: Signature of "connect" incompatible with supertype "BackendTest"

paroxython (https://github.com/laowantong/paroxython)
+ paroxython/assess_costs.py:85: error: Incompatible types in assignment (expression has type "_lru_cache_wrapper[int, [int], float]", variable has type "_lru_cache_wrapper_0[[int], float]")

pylint (https://github.com/pycqa/pylint)
+ pylint/checkers/classes/class_checker.py:2002: error: Returning Any from function declared to return "Union[SupportsDunderLT[Any], SupportsDunderGT[Any]]"  [no-any-return]

rich (https://github.com/Textualize/rich)
+ rich/progress_bar.py:145: error: Argument 3 to "__call__" of "_lru_cache_wrapper_0" has incompatible type "Optional[str]"; expected "str"  [arg-type]

zulip (https://github.com/zulip/zulip)
+ zerver/lib/cache.py:744: error: "_lru_cache_wrapper" expects 3 type arguments, but 1 given  [type-arg]
+ zerver/lib/cache.py:761: error: Unused "type: ignore" comment

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/connector.py:935: error: Too many arguments for "__call__" of "_lru_cache_wrapper_0"  [call-arg]
+ aiohttp/connector.py:941: error: Too many arguments for "__call__" of "_lru_cache_wrapper_0"  [call-arg]
+ aiohttp/connector.py:942: error: Too many arguments for "__call__" of "_lru_cache_wrapper_0"  [call-arg]

@asottile asottile closed this Sep 13, 2022
@asottile asottile deleted the functools-lru-cache-paramspec branch September 13, 2022 16:01
@asottile
Copy link
Contributor Author

this is going to go the same way as #7430 so I'm just going to retract it

@Greedquest
Copy link

Hello, has this been implemented somewhere. lru_cache and cache would really benefit from ParamSpec they seem like the textbook usecase. Right now mypy is seeing the function args as Any which means my callsites are not enforced.

@kkirsche
Copy link
Contributor

I don't believe so. I know when I tried it originally, I had run into some problems getting everything to work with the various dunder (double underscore) properties. Some of my personal obligations are clearing up so I may be able to take another stab at this. I may have still misunderstood ParamSpec at that time (I wanted it to be more like typescript function parameter utility type). At the very least, giving it another shot should allow me to articulate more clearly what specific issues I ran into in case others may be able to help solve those.

@JelleZijlstra
Copy link
Member

I believe the difficulty is in dealing with __get__ to make it work correctly as both a function and a method decorator.

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 this pull request may close these issues.

functools.lru_cache wrapped function with TypeVar has wrong return type.
6 participants