Fix MutableMapping.update to require string keys

[ndmitchell]

Currently with most type checkers you can do:

d: dict[int, int] = {}
d.update(a=2)

The reason for that is that update merely requires MutableMapping[_KT, _VT], whereas as soon as you are passing kwargs, you must have a str key type.

[AlexWaygood]

Could you also update the other methods that need to be kept in line with this method, as per this comment?

typeshed/stdlib/typing.pyi

Lines 790 to 800 in ec8b6ec

	# Various mapping classes have __ior__ methods that should be kept roughly in line with .update():
	# -- dict.__ior__
	# -- os._Environ.__ior__
	# -- collections.UserDict.__ior__
	# -- collections.ChainMap.__ior__
	# -- peewee.attrdict.__add__
	# -- peewee.attrdict.__iadd__
	# -- weakref.WeakValueDictionary.__ior__
	# -- weakref.WeakKeyDictionary.__ior__
	@overload
	defupdate(self, m: SupportsKeysAndGetItem[_KT, _VT], /, **kwargs: _VT) ->None: ...

[AlexWaygood]

Oh no scratch that, none of those other methods take keyword arguments

[github-actions]

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

aioredis (https://github.com/aio-libs/aioredis)
+ aioredis/client.py:146: note: def update(self, SupportsKeysAndGetItem[Any, Any], /) -> None+ aioredis/client.py:146: note: @overload+ aioredis/client.py:146: note: @overload+ aioredis/client.py:146: note: def update(self, Iterable[tuple[Any, Any]], /) -> None
operator (https://github.com/canonical/operator)
+ ops/model.py:1960: note: def update(self, SupportsKeysAndGetItem[str, str], /) -> None+ ops/model.py:1960: note: @overload+ ops/model.py:1960: note: @overload+ ops/model.py:1960: note: def update(self, Iterable[tuple[str, str]], /) -> None
ibis (https://github.com/ibis-project/ibis)
+ ibis/common/annotations.py:450: note: def update(self, SupportsKeysAndGetItem[Any, Any], /) -> None+ ibis/common/annotations.py:450: note: def update(self, Iterable[tuple[Any, Any]], /) -> None
mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ bson/son.py:146: error: Unused "type: ignore" comment [unused-ignore]
werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/datastructures/mixins.py:309: error: No overload variant of "update" of "MutableMapping" matches argument type "dict[str, V]" [call-overload]+ src/werkzeug/datastructures/mixins.py:309: note: Possible overload variants:+ src/werkzeug/datastructures/mixins.py:309: note: def update(self, SupportsKeysAndGetItem[K, V], /) -> None+ src/werkzeug/datastructures/mixins.py:309: note: def update(self, Iterable[tuple[K, V]], /) -> None
discord.py (https://github.com/Rapptz/discord.py)
+ discord/client.py:733: note: def update(self, SupportsKeysAndGetItem[str, int | None], /) -> None+ discord/client.py:733: note: def update(self, Iterable[tuple[str, int | None]], /) -> None+ discord/client.py:759: note: def update(self, SupportsKeysAndGetItem[str, int | None], /) -> None+ discord/client.py:759: note: def update(self, Iterable[tuple[str, int | None]], /) -> None
meson (https://github.com/mesonbuild/meson)
+ mesonbuild/options.py:1149:9: error: No overload variant of "update" of "MutableMapping" matches argument type "dict[str, UserBooleanOption | UserComboOption | UserFeatureOption | UserIntegerOption | UserStdOption | UserStringArrayOption | UserStringOption | UserUmaskOption]" [call-overload]+ mesonbuild/options.py:1149:9: note: Possible overload variants:+ mesonbuild/options.py:1149:9: note: def update(self, SupportsKeysAndGetItem[OptionKey, UserBooleanOption | UserComboOption | UserFeatureOption | UserIntegerOption | UserStdOption | UserStringArrayOption | UserStringOption | UserUmaskOption], /) -> None+ mesonbuild/options.py:1149:9: note: def update(self, Iterable[tuple[OptionKey, UserBooleanOption | UserComboOption | UserFeatureOption | UserIntegerOption | UserStdOption | UserStringArrayOption | UserStringOption | UserUmaskOption]], /) -> None

[AlexWaygood]

The only two new errors here are in werkzeug and meson. The werkzeug one already has a type: ignore comment on the method (just a different line) and the meson one has a FIXME: this method must be deleted comment immediately above it. So I think we're good to go!

[AlexWaygood]

• https://github.com/pallets/werkzeug/blob/99fc293bc7dccd6a7dc96b96012d4aa849794ba4/src/werkzeug/datastructures/mixins.py#L301-L311
• https://github.com/mesonbuild/meson/blob/c4431549478f1678cb014a10eccbcfd438d6bdfc/mesonbuild/options.py#L1147-L1149

[srittau]

Should this be:

defupdate(self: Mapping[str, _VT], m: SupportsKeysAndGetItem[str, _VT], /, **kwargs: _VT) ->None: ...

It's possible that _KT is constrained to str anyway via the self mapping, but I still think being explicit would be better (and easier on the type checkers).

[JelleZijlstra]

I think SupportsKeysAndGetItem is invariant in the first arg so that might cause trouble?

[ndmitchell]

#14100 for these cleanups.

[srittau]

Same here.

[QEDady]

Hello,

The new annotation will not allow the following, I think.

d = {}
d.update({1:2}, a=3)
d.update([(1, 2), a=3)

Shouldn't it be:

 def update(self: Mapping[str | _KT, _VT], m: SupportsKeysAndGetItem[str | _KT, _VT], /, **kwargs: _VT) -> None: ...
def update(self: Mapping[str | _KT, _VT], m: Iterable[tuple[str | _KT, _VT]], /, **kwargs: _VT) -> None: ...

?

[srittau]

We should probably add more test cases for these basic collection types, especially for more complex cases like MutableMapping.update.

[srittau]

That said, the following test case passes:

defcheck_update_method() ->None:
d: dict[int, int] = {}
d.update({1:2}, a=3)
d.update([(1, 2)], a=3)

But we need the explicit annotation for the dict.

[srittau]

In fact, while the mypy tests pass, the pyright tests don't: #14104