Skip to content

Conversation

@ikonst
Copy link
Contributor

@ikonst ikonst commented Apr 25, 2020

Per docs:
Since a dictionary is used to cache results, the positional and keyword arguments to the function must be hashable.

@hauntsaninja
Copy link
Collaborator

A little wary of this change; type checkers don't really understand Hashable: #3884 (comment)

@ikonst
Copy link
Contributor Author

ikonst commented Apr 26, 2020

Thanks, I learned something :) I've indeed wondered how Hashable worked in mypy since object is hashable.

I tried this approach of restricting arg types to Hashable, and mypy did catch (thanks to #3219) passing list, dict etc. which would otherwise raise in runtime. So it does seem to add some value without adding false negatives. no?

BTW, Another thing that irks me about this definition is that it doesn't maintain the wrapped function's signature, i.e. it's not

_T = TypeVar('_T', bound=Callable)

def lru_cache() -> Callable[[_T], _T]: ...

Of course defining it this way would do away with the wrapper's methods like cache_clear method, which I wouldn't want to do.

@JelleZijlstra
Copy link
Member

I might have been too pessimistic about Hashable in the past; mypy does understand it now. This PR looks like a step in the right direction.

@JelleZijlstra JelleZijlstra merged commit 1dc5853 into python:master May 28, 2020
vishalkuo pushed a commit to vishalkuo/typeshed that referenced this pull request Jun 26, 2020
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.

3 participants