Skip to content
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

Widen MutableMapping.update type #6653

Merged
merged 2 commits into from
Dec 22, 2021
Merged

Widen MutableMapping.update type #6653

merged 2 commits into from
Dec 22, 2021

Conversation

JelleZijlstra
Copy link
Member

Part of #6056

@github-actions
Copy link
Contributor

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

bidict (https://github.com/jab/bidict)
+ bidict/_mut.py:150: error: Signature of "update" incompatible with supertype "MutableMapping"  [override]
+ bidict/_mut.py:150: note:      Superclass:
+ bidict/_mut.py:150: note:          @overload
+ bidict/_mut.py:150: note:          def update(self, SupportsKeysAndGetItem[KT, VT], **kwargs: VT) -> None
+ bidict/_mut.py:150: note:          @overload
+ bidict/_mut.py:150: note:          def update(self, Iterable[Tuple[KT, VT]], **kwargs: VT) -> None
+ bidict/_mut.py:150: note:          @overload
+ bidict/_mut.py:150: note:          def update(self, **kwargs: VT) -> None
+ bidict/_mut.py:150: note:      Subclass:
+ bidict/_mut.py:150: note:          @overload
+ bidict/_mut.py:150: note:          def update(self, Mapping[KT, VT], **kw: VT) -> None
+ bidict/_mut.py:150: note:          @overload
+ bidict/_mut.py:150: note:          def update(self, Iterable[Tuple[KT, VT]], **kw: VT) -> None
+ bidict/_mut.py:150: note:          @overload
+ bidict/_mut.py:150: note:          def update(self, **kw: VT) -> None

websockets (https://github.com/aaugustin/websockets)
+ src/websockets/datastructures.py:137: error: Signature of "update" incompatible with supertype "MutableMapping"
+ src/websockets/datastructures.py:137: note:      Superclass:
+ src/websockets/datastructures.py:137: note:          @overload
+ src/websockets/datastructures.py:137: note:          def update(self, SupportsKeysAndGetItem[str, str], **kwargs: str) -> None
+ src/websockets/datastructures.py:137: note:          @overload
+ src/websockets/datastructures.py:137: note:          def update(self, Iterable[Tuple[str, str]], **kwargs: str) -> None
+ src/websockets/datastructures.py:137: note:          @overload
+ src/websockets/datastructures.py:137: note:          def update(self, **kwargs: str) -> None
+ src/websockets/datastructures.py:137: note:      Subclass:
+ src/websockets/datastructures.py:137: note:          def update(self, *args: Union[Headers, Mapping[str, str], Iterable[Tuple[str, str]]], **kwargs: str) -> None

werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/datastructures.pyi:99: error: Signature of "update" incompatible with supertype "MutableMapping"
+ src/werkzeug/datastructures.pyi:99: note:      Superclass:
+ src/werkzeug/datastructures.pyi:99: note:          @overload
+ src/werkzeug/datastructures.pyi:99: note:          def update(self, SupportsKeysAndGetItem[K, V], **kwargs: V) -> None
+ src/werkzeug/datastructures.pyi:99: note:          @overload
+ src/werkzeug/datastructures.pyi:99: note:          def update(self, Iterable[Tuple[K, V]], **kwargs: V) -> None
+ src/werkzeug/datastructures.pyi:99: note:          @overload
+ src/werkzeug/datastructures.pyi:99: note:          def update(self, **kwargs: V) -> None
+ src/werkzeug/datastructures.pyi:99: note:      Subclass:
+ src/werkzeug/datastructures.pyi:99: note:          def update(self, *args: Union[Mapping[K, V], Iterable[Tuple[K, V]]], **kwargs: V) -> None
+ src/werkzeug/datastructures.pyi:277: error: Signature of "update" incompatible with supertype "MutableMapping"
+ src/werkzeug/datastructures.pyi:277: note:      Superclass:
+ src/werkzeug/datastructures.pyi:277: note:          @overload
+ src/werkzeug/datastructures.pyi:277: note:          def update(self, SupportsKeysAndGetItem[str, str], **kwargs: str) -> None
+ src/werkzeug/datastructures.pyi:277: note:          @overload
+ src/werkzeug/datastructures.pyi:277: note:          def update(self, Iterable[Tuple[str, str]], **kwargs: str) -> None
+ src/werkzeug/datastructures.pyi:277: note:          @overload
+ src/werkzeug/datastructures.pyi:277: note:          def update(self, **kwargs: str) -> None
+ src/werkzeug/datastructures.pyi:277: note:      Subclass:
+ src/werkzeug/datastructures.pyi:277: note:          def update(self, *args: Union[Mapping[str, Union[str, int]], Iterable[Tuple[str, Union[str, int]]]], **kwargs: Union[Union[str, int], Iterable[Union[str, int]]]) -> None

rclip (https://github.com/yurijmikhalevich/rclip)
- rclip/db.py:70: error: Argument 1 to "update" of "MutableMapping" has incompatible type "NewImage"; expected "Mapping[Any, None]"
+ rclip/db.py:70: error: Argument 1 to "update" of "MutableMapping" has incompatible type "NewImage"; expected "SupportsKeysAndGetItem[Any, None]"
+ rclip/db.py:70: note: Following member(s) of "NewImage" have conflicts:
+ rclip/db.py:70: note:     Expected:
+ rclip/db.py:70: note:         def __getitem__(self, Any) -> None
+ rclip/db.py:70: note:     Got:
+ rclip/db.py:70: note:         def __getitem__(self, str) -> object

ibis (https://github.com/ibis-project/ibis)
- ibis/backends/clickhouse/registry.py:720: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[Type[ValueOp], Any]"; expected "Mapping[Type[Node], Any]"
+ ibis/backends/clickhouse/registry.py:720: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[Type[ValueOp], Any]"; expected "SupportsKeysAndGetItem[Type[Node], Any]"
- ibis/backends/clickhouse/registry.py:721: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Callable[[Any, Any, VarArg(Any)], Any]]"; expected "Mapping[Type[Node], Any]"
+ ibis/backends/clickhouse/registry.py:721: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Callable[[Any, Any, VarArg(Any)], Any]]"; expected "SupportsKeysAndGetItem[Type[Node], Any]"
- ibis/backends/clickhouse/registry.py:722: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[Type[UnaryOp], Callable[[Any, Any], Any]]"; expected "Mapping[Type[Node], Any]"
+ ibis/backends/clickhouse/registry.py:722: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[Type[UnaryOp], Callable[[Any, Any], Any]]"; expected "SupportsKeysAndGetItem[Type[Node], Any]"
- ibis/backends/clickhouse/registry.py:723: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Any]"; expected "Mapping[Type[Node], Any]"
+ ibis/backends/clickhouse/registry.py:723: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Any]"; expected "SupportsKeysAndGetItem[Type[Node], Any]"
- ibis/backends/base/sql/registry/main.py:361: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Any]"; expected "Mapping[Type[Node], Any]"
+ ibis/backends/base/sql/registry/main.py:361: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Any]"; expected "SupportsKeysAndGetItem[Type[Node], Any]"
- ibis/backends/impala/compiler.py:25: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Any]"; expected "Mapping[Type[Node], Any]"
+ ibis/backends/impala/compiler.py:25: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[AnnotableMeta, Any]"; expected "SupportsKeysAndGetItem[Type[Node], Any]"

aioredis (https://github.com/aio-libs/aioredis)
- aioredis/client.py:4139: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[Union[bytes, str], Optional[Any]]"; expected "Mapping[Union[bytes, str, memoryview], Callable[[Dict[str, str]], None]]"  [arg-type]
+ aioredis/client.py:4139: error: Argument 1 to "update" of "MutableMapping" has incompatible type "Dict[Union[bytes, str], Optional[Any]]"; expected "SupportsKeysAndGetItem[Union[bytes, str, memoryview], Callable[[Dict[str, str]], None]]"  [arg-type]

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Primer shows that some third-party sub-classes need to be updated. That is ok.

@srittau srittau merged commit 14268f6 into master Dec 22, 2021
@srittau srittau deleted the JelleZijlstra-patch-2 branch December 22, 2021 07:54
@aaugustin
Copy link
Contributor

Third-party maintainer (of websockets) ends ups here after half an hour spent tracking down what changed and is puzzled about how to fix this :-) What would you recommend?

Here's a trimmed down version of the code that is now raising a warning:

from typing import Iterable, Mapping, MutableMapping, Tuple, Union

class Headers(MutableMapping[str, str]):
    def update(
        self,
        *args: Union[Headers, Mapping[str, str], Iterable[Tuple[str, str]]],
        **kwargs: str,
    ) -> None:
        ...

I'd like to avoid introducing a dependency on typeshed or if typing.TYPE_CHECKING: ... conditionals — I don't have any at this point.

@aaugustin
Copy link
Contributor

(If there's no simple fix, this will get a # type: ignore, given the dubious effort / benefit ratio of doing anything else.)

aaugustin added a commit to python-websockets/websockets that referenced this pull request Mar 20, 2022
@JelleZijlstra
Copy link
Member Author

So the problem with your version is that MutableMapping.update accepts any object that provides the keys and __getitem__ methods, but your override only accepts Mapping objects. You can see that in the default implementation of update(): https://github.com/python/cpython/blob/9d1c4d69dbc800ac344565119337fcf490cdc800/Lib/_collections_abc.py#L941

The most prominent example of an object that has keys and __getitem__ but isn't a Mapping is the pandas.Series class.

Possible fixes:

  • Use _typeshed.SupportsKeysAndGetItem, but you'll have to import it within an if TYPE_CHECKING block because it's stub-only.
  • Define your own version of the protocol in your code. It's pretty simple:
    class SupportsKeysAndGetItem(Protocol[_KT, _VT_co]):
  • Don't override update() at all, and rely on the stdlib's MutableMapping.update implementation.

@aaugustin
Copy link
Contributor

Thank you very much!

I will investigate option 2; I didn't notice the type was that simple. (And double check that the implementation indeed supports the wider type; I think it does.)

aaugustin added a commit to python-websockets/websockets that referenced this pull request Mar 21, 2022
Restore compatibility with the latest mypy.

Refs python/typeshed#6653.
@aaugustin
Copy link
Contributor

Fixed. Thanks again! :-)

aaugustin added a commit to python-websockets/websockets that referenced this pull request Mar 23, 2022
Restore compatibility with the latest mypy.

Refs python/typeshed#6653.
DMRobertson pushed a commit to matrix-org/synapse that referenced this pull request May 6, 2022
Mypy now includes python/typeshed#6653
which triggered

```
stubs/sortedcontainers/sorteddict.pyi:88: error: Signature of "update" incompatible with supertype "MutableMapping"  [override]
stubs/sortedcontainers/sorteddict.pyi:88: note:      Superclass:
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, SupportsKeysAndGetItem[_KT, _VT], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, Iterable[Tuple[_KT, _VT]], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:      Subclass:
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, Mapping[_KT, _VT], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, Iterable[Tuple[_KT, _VT]], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, **kwargs: _VT) -> None
```
DMRobertson pushed a commit to matrix-org/synapse that referenced this pull request May 6, 2022
Mypy now includes python/typeshed#6653
which triggered

```
stubs/sortedcontainers/sorteddict.pyi:88: error: Signature of "update" incompatible with supertype "MutableMapping"  [override]
stubs/sortedcontainers/sorteddict.pyi:88: note:      Superclass:
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, SupportsKeysAndGetItem[_KT, _VT], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, Iterable[Tuple[_KT, _VT]], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:      Subclass:
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, Mapping[_KT, _VT], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, Iterable[Tuple[_KT, _VT]], **kwargs: _VT) -> None
stubs/sortedcontainers/sorteddict.pyi:88: note:          @overload
stubs/sortedcontainers/sorteddict.pyi:88: note:          def update(self, **kwargs: _VT) -> None
```
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