[email protected] regression: Argument 1 to "get" of "Mapping" has incompatible type

[sobolevn]

I got a new regression from the latest release on a real project: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/logic/arguments/function_args.py#L125-L126

Error:

wemake_python_styleguide/logic/arguments/function_args.py:126: error: Argument 1 to "get" of "Mapping" has incompatible type "Optional[Any]"; expected "str"

Simplier repro:

from typing import Mapping, Any, Optional

x: Optional[Any]
m: Mapping[str, int]

m.get(x)
# error: Argument 1 to "get" of "Mapping" has incompatible type "Optional[Any]"; expected "str"

So, what do you think: is this a valid error? Because it will work at runtime with no problem. And since it has Any part in it, sometimes it can even be str. So, in my app it was working as expected in all cases: if x is str and exists in m - then fine. If not - then just return None.

I am openning it here, because it looks like a typeshed issue, rather than a mypy issue.

[Akuli]

typeshed/stdlib/typing.pyi

Lines 459 to 462 in 9aa66f0

	@overload
	def get(self, key: _KT) -> _VT_co | None: ...
	@overload
	def get(self, __key: _KT, __default: _VT_co | _T) -> _VT_co | _T: ...

We should probably change _KT to _KT | None in both overloads. Technically the first argument can be anything that overlaps with _KT, but in practice, allowing None is probably good enough.

[srittau]

The stricter typing for get() will catch potential type problems. While your example will work at runtime, it indicates a likely bug as None is not a valid key. I like the new behavior, although I could be convinced otherwise. Maybe we could also just special case None.

[jab]

I hit this while working on my bidict library, which implements bidirectional mapping data structures which wrap two (regular one-directional) mappings, and which is type hinted and checked with mypy.

it indicates a likely bug

I disagree that this indicates a "likely" bug. It could be a bug. But it's just as likely that there is no bug. And the current type annotations are flagging perfectly correct and idiomatic code:

Sometimes you have no idea what type x is. One of the main reasons that d.get(x[, default]) and d.pop(x, default) exist -- with the optional default argument -- is to support coding in a "check the result after calling" style, rather than the "look before you leap" style, which is often less Pythonic.

Yet these APIs' type hints are treating these APIs the same as Mapping.__getitem__ and MutableMapping.__delitem__ respectively, even though they are not meant to be used the same way.

Would you accept a PR that changes the Mapping.get(x[, default]) and MutableMapping.pop(x, default) type hints to support the "check result after calling" style they're intended to support? Note this is more than just special-casing None.

Thank you for your consideration.

[srittau]

I don't think that this is the way forward. Type checkers are supposed to check that the types are correct after all. If you don't know types or are not interested in the extra type checking the annotations provide, you can always use the Any escape hatch.

[Akuli]

We could probably make it work well enough in practice by special-casing Nones.