Skip to content

Conversation

@max-muoto
Copy link
Contributor

@max-muoto max-muoto commented Jul 1, 2024

As detailed in #8182, the stubs for memoryview currently represent a sequence of ints, while at runtime this vary across ints, bytes, floats, and booleans at runtime after casting.

This PR fixes this by making MemoryView generic, while using 3.13 typevar defaults to avoid any type of large-scale regression, and adding appropiate overloads when casting.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@max-muoto max-muoto changed the title Make MemoryView Generic Make MemoryView Generic, make cast accurate Jul 1, 2024
@JelleZijlstra
Copy link
Member

Sounds like you found a mypy bug! Would be good to minimize it and report it over on the mypy repo.

@github-actions

This comment has been minimized.

@max-muoto max-muoto marked this pull request as ready for review July 1, 2024 03:30
def __buffer__(self, flags: int, /) -> memoryview: ...
def __release_buffer__(self, buffer: memoryview, /) -> None: ...

_IntegerFormats: TypeAlias = Literal[
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if we'd prefer these to be inline for the sake of IDEs, seemed harder to maintain so just decided to place them outside.

@max-muoto
Copy link
Contributor Author

Most of the errors:

- aioredis/connection.py:933: error: Unsupported right operand type for in ("bytes | memoryview | int")  [operator]
+ aioredis/connection.py:933: error: Unsupported right operand type for in ("bytes | memoryview[int] | int")  [operator]
- aioredis/connection.py:934: error: Item "memoryview" of "bytes | memoryview | int" has no attribute "split"  [union-attr]
+ aioredis/connection.py:934: error: Item "memoryview[int]" of "bytes | memoryview[int] | int" has no attribute "split"  [union-attr]
- aioredis/connection.py:934: error: Item "int" of "bytes | memoryview | int" has no attribute "split"  [union-attr]
+ aioredis/connection.py:934: error: Item "int" of "bytes | memoryview[int] | int" has no attribute "split"  [union-attr]
- aioredis/client.py:4114: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview, Any | None]", variable has type "dict[bytes | str | memoryview, Callable[[dict[str, str]], Awaitable[None]]]")  [assignment]
+ aioredis/client.py:4114: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview[int], Any | None]", variable has type "dict[bytes | str | memoryview[int], Callable[[dict[str, str]], Awaitable[None]]]")  [assignment]
- aioredis/client.py:4158: error: Argument 1 to "update" of "MutableMapping" has incompatible type "dict[bytes | str | memoryview, Any | None]"; expected "SupportsKeysAndGetItem[bytes | str | memoryview, Callable[[dict[str, str]], Awaitable[None]]]"  [arg-type]
+ aioredis/client.py:4158: error: Argument 1 to "update" of "MutableMapping" has incompatible type "dict[bytes | str | memoryview[int], Any | None]"; expected "SupportsKeysAndGetItem[bytes | str | memoryview[int], Callable[[dict[str, str]], Awaitable[None]]]"  [arg-type]
- aioredis/client.py:4172: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview, Callable[[dict[str, str]], Awaitable[None]]]", variable has type "dict[Any, Any | None]")  [assignment]
+ aioredis/client.py:4172: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview[int], Callable[[dict[str, str]], Awaitable[None]]]", variable has type "dict[Any, Any | None]")  [assignment]

Are not net-new (just memoryview -> memoryview[int]).

For web.py in aiohttp,

class TCPSite(BaseSite):
    __slots__ = ("_host", "_port", "_reuse_address", "_reuse_port")

    def __init__(
        self,
        runner: "BaseRunner",
        host: Optional[str] = None,
        port: Optional[int] = None,
        *,

We see that the isinstance check should only narrow to str assuming TCPSite has accurate types.

@max-muoto
Copy link
Contributor Author

max-muoto commented Jul 1, 2024

Sounds like you found a mypy bug! Would be good to minimize it and report it over on the mypy repo.

Are you referring to one of the previous crashes, or something else? Or perhaps the aiohttp error not getting caught before.

@github-actions

This comment has been minimized.

@max-muoto max-muoto changed the title Make MemoryView Generic, make cast accurate Make MemoryView Generic, make cast accurate Jul 1, 2024
@JelleZijlstra
Copy link
Member

Sounds like you found a mypy bug! Would be good to minimize it and report it over on the mypy repo.

Are you referring to one of the previous crashes, or something else? Or perhaps the aiohttp error not getting caught before.

I'm referring to the crash.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2024

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

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/web.py:353:25: error: Argument 2 to "TCPSite" has incompatible type "str | memoryview[Any]"; expected "str | None"  [arg-type]

aioredis (https://github.com/aio-libs/aioredis)
- aioredis/connection.py:933: error: Unsupported right operand type for in ("bytes | memoryview | int")  [operator]
+ aioredis/connection.py:933: error: Unsupported right operand type for in ("bytes | memoryview[int] | int")  [operator]
- aioredis/connection.py:934: error: Item "memoryview" of "bytes | memoryview | int" has no attribute "split"  [union-attr]
+ aioredis/connection.py:934: error: Item "memoryview[int]" of "bytes | memoryview[int] | int" has no attribute "split"  [union-attr]
- aioredis/connection.py:934: error: Item "int" of "bytes | memoryview | int" has no attribute "split"  [union-attr]
+ aioredis/connection.py:934: error: Item "int" of "bytes | memoryview[int] | int" has no attribute "split"  [union-attr]
- aioredis/client.py:4114: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview, Any | None]", variable has type "dict[bytes | str | memoryview, Callable[[dict[str, str]], Awaitable[None]]]")  [assignment]
+ aioredis/client.py:4114: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview[int], Any | None]", variable has type "dict[bytes | str | memoryview[int], Callable[[dict[str, str]], Awaitable[None]]]")  [assignment]
- aioredis/client.py:4158: error: Argument 1 to "update" of "MutableMapping" has incompatible type "dict[bytes | str | memoryview, Any | None]"; expected "SupportsKeysAndGetItem[bytes | str | memoryview, Callable[[dict[str, str]], Awaitable[None]]]"  [arg-type]
+ aioredis/client.py:4158: error: Argument 1 to "update" of "MutableMapping" has incompatible type "dict[bytes | str | memoryview[int], Any | None]"; expected "SupportsKeysAndGetItem[bytes | str | memoryview[int], Callable[[dict[str, str]], Awaitable[None]]]"  [arg-type]
- aioredis/client.py:4172: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview, Callable[[dict[str, str]], Awaitable[None]]]", variable has type "dict[Any, Any | None]")  [assignment]
+ aioredis/client.py:4172: error: Incompatible types in assignment (expression has type "dict[bytes | str | memoryview[int], Callable[[dict[str, str]], Awaitable[None]]]", variable has type "dict[Any, Any | None]")  [assignment]

@max-muoto
Copy link
Contributor Author

Sounds like you found a mypy bug! Would be good to minimize it and report it over on the mypy repo.

Are you referring to one of the previous crashes, or something else? Or perhaps the aiohttp error not getting caught before.

I'm referring to the crash.

Sounds good, I'll put up an issue in that case.

@max-muoto
Copy link
Contributor Author

Just wanted to bump this, and see if anyone was interested in reviewing?

@JelleZijlstra JelleZijlstra merged commit 4316e00 into python:main Jul 12, 2024
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.

2 participants