-
Notifications
You must be signed in to change notification settings - Fork 237
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
Reconsider making Mapping covariant in the key type #445
Comments
I think this is a crucial point. I don't see a good solution without adding some internal special-casing in type checkers. |
Pinging @rowillia, since he had issues with invariance recently. |
@JukkaL def func(arg: BaseMap[str, int]) -> int: ...
x: Dict[Union[str, unicode], int]
func(x) # people want this to work, but it doesn't with invariant Mapping instead of BaseMap |
It's unclear to me how useful |
Actually the original proposal in http://bugs.python.org/issue25988 was to add an ABC with two methods |
Well, it's probably only worth adding such a protocol if it's useful often enough -- defining one manually is easy in case it would only be used occasionally. I don't know whether this is the case, though. |
OK, right, let us postpone this then. It will be easy to add one later, if we will see that it is actually needed. |
This is small enough that it probably won't need to be mentioned. |
Hi, this might add to the cases where the key not being covariant fails, not sure: from __future__ import annotations
from typing import *
A=TypeVar('A',bound=Hashable)
B=TypeVar('B')
class C(MutableMapping[A, B], Generic[A, B]):
def __init__(self):
self.d = cast(Dict[A,B],dict())
def test(self,a:A):
return self.d.get(1) #should not typecheck |
I believe it's now too late to fix this. |
I encountered this issue when adding type annotations to Streamlit. I came across a function that looked like def lower_clean_dict_keys(dict):
return {k.lower().strip(): v for k, v in dict.items()} Annotating this function, I just followed my instincts based on naming, and annotated it as _Value = TypeVar("_Value")
def lower_clean_dict_keys(dict: Dict[str, _Value]) -> Dict[str, _Value]:
return {k.lower().strip(): v for k, v in dict.items()} Mypy quickly pointed out that
My next attempt looked like this: _Value = TypeVar("_Value")
def lower_clean_dict_keys(dict: Mapping[str, _Value]) -> Mapping[str, _Value]:
return {k.lower().strip(): v for k, v in dict.items()} Imagine my surprise when this did not work out for the same reason as with I never found my covariant mapping type. Instead, I landed on this solution: _Key = TypeVar("_Key", bound=str)
_Value = TypeVar("_Value")
def lower_clean_dict_keys(dict: Mapping[_Key, _Value]) -> Dict[str, _Value]:
return {k.lower().strip(): v for k, v in dict.items()} Here, I add a free type variable for the key, effectively making the mapping covariant. For the purposes of type checking, this covers what I need, but I don't think it is elegant. It forces me to introduce yet another type variable (which, confusingly, is only used once*). The issue here, really, is that Given that
* The "normal" pattern is to use it at least twice, in order to indicate dependency between types. TL;DR: Given my experience, I desire better documentation for the |
Better documentation for typing would be desirable overall and there is some first ideas in the typing repository: https://github.com/python/typing/issues?q=is%3Aopen+is%3Aissue+label%3A%22topic%3A+documentation%22 We have some very tight protocols in from typing import TYPE_CHECKING
if TYPE_CHECKING:
from _typeshed import SupportsItems We are currently working to bring some of these types into |
Not sure whether it belongs here, but it's quite reasonable to implement "type based callback map" in this way: from typing import Any, TypeVar, Callable
class MyType:
pass
_T = TypeVar("_T", bound=MyType)
class TypeA(MyType):
content: str
class TypeB(MyType):
content: int
class Case:
def __init__(self, cb: dict[type[_T], Callable[[_T], Any]]):
pass
Case({
TypeA: lambda a: a.content + "!!!", # error: Operator "+" not supported for types "str | int" and "str"
TypeB: lambda b: b.content + 5, # error: Operator "+" not supported for types "str | int" and "int"
}) Type checker assumed that the key type of However it tried to apply |
…n the key type See: - python/typing#445 - python/typing#273
Confusion with invariant container types is one of the more common questions mypy users have. Often these can be easily worked around by using a covariant type such as
Sequence
instead ofList
. However, there is no fully covariant dictionary-like type --Mapping
is invariant in the key type, since the key type is used as an argument type of__getitem__
andget
. This is particularly problematic in Python 2, as mappings withunicode
keys are common, and a mapping withstr
keys is not compatible with them, even though they are fine at runtime (at least if the keys are ascii only).I can see a few things we could do.
1) Don't care about unsafety
We'd make
Mapping
covariant even though it's known to be unsafe (#273). The argument is that the potential unsafety is less of a problem than the confusing user experience. This wouldn't help with certain related things like using aunicode
key to accessMapping[str, X]
, which is arguably okay.Note that this was proposed and rejected earlier.
2) Use
object
inMapping
argument typesWe'd make
Mapping
covariant and useobject
in the argument type of__getitem__
andget
. This would result in less effective type checking, but I'd consider this to still be correct at least forget
, since it can always fall back to the default. Also,__getitem__
would typically only generate aKeyError
if given a key of an unrelated type, and this is arguably not a type error. Indexing operations on mappings can already fail withKeyError
, and this isn't considered a type safety issue.We could also recommend that type checkers special case type checking of
Mapping.__getitem__
andMapping.get
(and also the corresponding methods in subclasses such asDict
). A reasonable rule would be to require that the actual key type in code liked[k]
overlaps with the declared key type. This would allow things like usingunicode
keys forMapping[str, int]
, which should perhaps be fine in Python 2. We could still reject things liked[1]
if the declared key type isstr
, sinceint
andstr
are not overlapping.Subclasses of
Mapping
that only support specific runtime key types would now be rejected. For example, this wouldn't work any more:However, it would be easy enough to refactor this to type check:
I think that this is reasonable -- the required assert (or cast) makes it clear that this is potentially unsafe.
3) Just teach users to work around using unions or type variables
Maybe keeping
Mapping
is invariant in the key type is reasonable. Often the lack of covariance can be worked around by using other type system features. Here are some examples:Union[Mapping[str, int], Mapping[unicode, int]]
instead ofMapping[unicode, int]
.Mapping[AnyStr, int]
in Python 2 andMapping[str, int]
in Python 3 using a conditional type alias.AnyText = TypeVar('AnyText', str, Text)
and useMapping[AnyText, int]
. This is a little awkward, since in Python 3 the values of the type variable would bestr
andstr
.I don't like these for a few reasons:
Sequence
.Mapping[unicode, int]
.The text was updated successfully, but these errors were encountered: