-
Notifications
You must be signed in to change notification settings - Fork 288
-
While trying to create type stubs for asyncache, I came across this difference in behaviour between mypy and pyright.
The following code is accepted in the pyright playground
from collections.abc import Hashable from typing import Protocol, TypeVar, MutableMapping def hashkey(*args: Hashable, **kwargs: Hashable) -> tuple[Hashable, ...]: return tuple(a for a in args) _RT_co = TypeVar('_RT_co', covariant=True) class CacheableFunction(Protocol[_RT_co]): @staticmethod def __call__(*args: Hashable, **kwargs: Hashable) -> _RT_co: ... _KT = TypeVar('_KT', covariant=True) _T = TypeVar('_T') def test_fun( cache: MutableMapping[_KT, _T] = {}, hashfun: CacheableFunction[_KT] = hashkey, *args: Hashable ) -> _T: return cache[hashfun(args)] test: CacheableFunction = hashkey
However, mypy gives the error
Incompatible default for argument "hashfun" (default has type "Callable[[VarArg(Hashable), KwArg(Hashable)], tuple[Hashable, ...]]", argument has type "CacheableFunction[_KT]") [assignment]
main.py:19: note: "CacheableFunction[_KT].__call__" has type "Callable[[VarArg(Hashable), KwArg(Hashable)], _KT]"
Is this a bug? If so, of which type checker?
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 2 comments 9 replies
-
This looks like a bug in mypy. If you change the last line to test: CacheableFunction[tuple[Hashable, ...]] = hashkey, you can see that mypy accepts this assignment without error.
Beta Was this translation helpful? Give feedback.
All reactions
-
This error makes sense to me, since even though CacheableFunction is covariant the key type for Mapping isn't, so it's technically not type safe to accept hashkey for CachableFunction[_KT], since _KT could be something other than tuple[Hashable, ...] if you supply your own mapping without also supplying a hashfun.
You can add some overloads to deal with this:
@overload def test_fun( cache: MutableMapping[tuple[Hashable, ...], _T] = {}, hashfun: CacheableFunction[tuple[Hashable, ...]] = hashkey, *args: Hashable ) -> _T: ... @overload def test_fun( cache: MutableMapping[_KT, _T] , hashfun: CacheableFunction[_KT], *args: Hashable ) -> _T: ... def test_fun( cache: MutableMapping[Any, _T] = {}, hashfun: CacheableFunction[Any] = hashkey, *args: Hashable ) -> _T:
As a side note, I don't think Hashable is very useful for type checking since types that fulfill the Protocol can still be unhashable such as a tuple containing an unhashable object. I've found that it's usually not worth the hassle and it's better to just go with object or in this case probably Any, since arguments are contravariant.
Beta Was this translation helpful? Give feedback.
All reactions
-
once you fix
_KTyou can construct examples where the default is no longer compatible
_KT doesn't get a value until the generic function is invoked in a call expression. If there's any error here, I would expect it to be reported at the call site, not the function definition site. That's how pyright implements this support. Mypy could do the same.
since the default values are not part of the signature and thus not considered when checking calls
Yes, if the default value is not supplied (e.g. is specified as ..., as it sometimes is within stub files), then a type checker would not be able to verify compatibility at the call site. However, in real code (non-stubs) the default argument is always provided and can be used. There is also a trend to supply default argument values in stubs as well because they are valuable for language servers.
There are multiple active issues in the mypy issue tracker related to this use case. The earliest one is this one, which has been open for 7 years and has more than 80 upvotes. Regardless of whether mypy maintainers consider this a feature request or a bug, I think this is an issue with mypy specifically. I don't think there's a strong justification in the type system for generating an error here.
Beta Was this translation helpful? Give feedback.
All reactions
-
I think both ways are valid approaches, if the default values aren't considered at the call-site then we had better made sure the definition site is strict enough to catch potential errors involving default values, but when they are considered, you can relax this restriction. So it's more of an implementation detail for how you prevent this error and less about it being faulty behavior. The spec could rectify this by specifying how default values should be treated at definition and call time, including a recommendation when the default value has been ellided (this can also happen in overloads without error, not just in stubs, although in that case the value should usually be inferable from the implementation's signature).
FWIW I do think pyright's approach is better when we know the default value, because it ensures safety even if we made a consistency mistake in the definition that wasn't caught.
Beta Was this translation helpful? Give feedback.
All reactions
-
Thinking on it a bit further, there is one thing where pyright's approach is inherently less type safe, unless you apply complex logic and that is when you cast a callable with optional parameters to another compatible callable (based solely on the argument types) where some of those parameters are missing. So now you are stripping the required information that allowed to ensure type consistency at call-time while simultaneously encouraging a scenario where type consistency is violated, so there would need to be some logic at the cast-site to detect safety holes, so mypy's approach is more secure there, since it tries to ensure that you can't even write a definition with this class of problems, which is less ergonomic and more annoying to use, but I can also see why you would choose to nevertheless go this way, since it avoids introducing safety holes or complex logic at type analysis boundaries.
Beta Was this translation helpful? Give feedback.
All reactions
-
I'm not sure what you mean by "cast" in this context. Are you referring to a call to typing.cast? That's inherently unsafe since it subverts all type checks. If that's not what you mean, perhaps an example would help clarify.
Beta Was this translation helpful? Give feedback.
All reactions
-
I mean an implicit cast, i.e. anywhere you would perform a subtype or supertype check for compatibility between two types. Which mypy still performs in the case of typing.cast, it doesn't allow arbitrary incompatible casts without ignoring a type error.
Code sample in pyright playground
from collections.abc import Callable default_cache: dict[str, int] = {} def foo[T](x: T, y: dict[str, T] = default_cache) -> T: ... def takes_identity[T](func: Callable[[T], T], value: T) -> None: func(value) takes_identity(foo, "unsafe") # no error
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1