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

BufferedWriter is not compatible with SupportsWrite[AnyStr] #5243

Closed
tmke8 opened this issue Jun 7, 2023 · 13 comments · Fixed by #5272
Closed

BufferedWriter is not compatible with SupportsWrite[AnyStr] #5243

tmke8 opened this issue Jun 7, 2023 · 13 comments · Fixed by #5272
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@tmke8
Copy link

tmke8 commented Jun 7, 2023

Describe the bug
Sorry for the rambling title – I couldn't think of a good description of this:

import shutil
from typing import TYPE_CHECKING, AnyStr
from io import BufferedWriter

if TYPE_CHECKING:
    from _typeshed import SupportsRead, SupportsWrite


def f(s: SupportsRead[AnyStr], t: SupportsWrite[AnyStr]) -> None:  # with TypeVar
    shutil.copyfileobj(s, t)


def g(s: SupportsRead[bytes], t: SupportsWrite[bytes]) -> None:  # with concrete type
    shutil.copyfileobj(s, t)


def h(src: SupportsRead[bytes], tgt: BufferedWriter) -> None:
    f(src, tgt)  # E: Argument of type "BufferedWriter" cannot be assigned to parameter "t" of type "SupportsWrite[AnyStr@f]" in function "f"
    g(src, tgt)

Expected behavior
If I can pass tgt succesfully to g, then I don't see why it would fail with f. Clearly, BufferedWriter is compatible with SupportsWrite[bytes].

VS Code extension or command-line
command-line

Additional context
This happened after updating to the most recent version. I believe there were typeshed changes around this code.

@tmke8
Copy link
Author

tmke8 commented Jun 7, 2023

Actually, this is probably a duplicate of #5027.

@erictraut
Copy link
Collaborator

I don't think this is related to #5027. It doesn't involve a protocol implementation with a method-local TypeVar.

This looks like a problem with recent changes in typeshed.

Let's look more closely at your example. Function f passes an argument of type SupportsRead[bytes] for the first parameter. That means the AnyStr@f type variable will be constrained to bytes, which means that the second argument must be compatible with the type SupportsWrite[bytes], which is a protocol that has a method write. Once specialized, this method has the type def write(self, __s: bytes) -> object: .... The second argument that you're passing to f is of type BufferedWriter. It also has a method called write, but the type is def write(self, __buffer: ReadableBuffer) -> int: .... The type ReadableBuffer is a protocol that requires the type to have a method called __buffer__, but the bytes class doesn't have any such method. That means the two write methods are incompatible.

I presume this typeshed change was prompted by PEP 688.

I'm tempted to revert the typeshed update until we can determine what's going on here. This is likely to break a lot of code. I just confirmed that it breaks the pandas-stubs project, for example.

@JelleZijlstra, perhaps you can provide some insights here?

@JelleZijlstra
Copy link
Contributor

but the bytes class doesn't have any such method.

We in fact added __buffer__ to bytes across all versions, as discussed in python/typeshed#10224. It's here in typeshed: https://github.com/python/typeshed/blob/786bd22343a66ecdb0eeb3fe053dfb81365608c5/stdlib/builtins.pyi#L713 .

@erictraut
Copy link
Collaborator

Ah, I missed that. OK, then this might be a type analysis bug. I'll dig deeper. Thanks for the quick response.

@erictraut erictraut added the needs investigation Requires additional investigation to determine course of action label Jun 7, 2023
@erictraut
Copy link
Collaborator

erictraut commented Jun 7, 2023

I've reverted the typeshed changes for the time being and published a new version of pyright (1.1.313) that addresses this regression. I wanted to do this prior to this week's build of pylance (which is done on Wednesday morning's). This will give me time to investigate the underlying cause of this problem prior to next week's build.

@erictraut
Copy link
Collaborator

After looking at this more closely, I don't think this is a bug in pyright. Here's a self-contained, minimal code sample that demonstrates the problem:

from typing import AnyStr, Protocol, TypeVar

_T_co = TypeVar("_T_co", covariant=True)
_T_contra = TypeVar("_T_contra", contravariant=True)

class Buffer(Protocol):
    def __buffer__(self, __flags: int) -> memoryview:
        ...

class SupportsRead(Protocol[_T_co]):
    def read(self, __length: int = ...) -> _T_co:
        ...

class SupportsWrite(Protocol[_T_contra]):
    def write(self, __s: _T_contra) -> object:
        ...

class BufferedWriter:
    def write(self, __buffer: Buffer) -> int:
        raise NotImplementedError

def f(s: SupportsRead[AnyStr], t: SupportsWrite[AnyStr]) -> None:
    ...

def h(src: SupportsRead[bytes], tgt: BufferedWriter) -> None:
    f(src, tgt)

The issue is that BufferedWriter is not type compatible with SupportsWrite[AnyStr]. That's because AnyStr can be a str, and that's not compatible with Buffer. Mypy also emits an error for this code.

@JelleZijlstra, I don't fully understand the recent typeshed changes, but they appear to be creating a lot of new errors in existing typed code bases. I presume that you ran mypy_primer with the changes. I'm curious whether that indicated any significant problems for existing code bases? I'm hesitant to upgrade the bundled stubs for pyright until we've worked through some of these issues.

Another issue that seems to be related to this one is in the pandas-stubs project. It expects the type io.BytesIO to be type compatible with WriteBuffer[bytes] | WriteBuffer[str] where WriteBuffer is an internally-defined protocol class.

import io
from typing import Any, Protocol, TypeVar
AnyStr_contra = TypeVar("AnyStr_contra", str, bytes, contravariant=True)

class BaseBuffer(Protocol):
    @property
    def mode(self) -> str: ...
    def seek(self, __offset: int, __whence: int = ...) -> int: ...
    def seekable(self) -> bool: ...
    def tell(self) -> int: ...

class WriteBuffer(BaseBuffer, Protocol[AnyStr_contra]):
    def write(self, __b: AnyStr_contra) -> Any: ...
    def flush(self) -> Any: ...

def test(b: io.BytesIO):
    x: WriteBuffer[bytes] | WriteBuffer[str] = b # Type error with new typeshed

Let me know if you'd like me to open individual issues in the typeshed project to work through these issues (and others I'm seeing).

@JelleZijlstra
Copy link
Contributor

The issue is that BufferedWriter is not type compatible with SupportsWrite[AnyStr]. That's because AnyStr can be a str, and that's not compatible with Buffer. Mypy also emits an error for this code.

I think this should pass: in the call to f(), AnyStr should be solved to bytes, and then BufferedWriter is compatible with SupportsWrite[bytes].

Mypy emits an error here because in its bundled stubs, bytes doesn't yet have a __buffer__ method. However, if you change Buffer to some other method that bytes has and str doesn't, it succeeds: https://mypy-play.net/?mypy=latest&python=3.12&gist=6ade2ba7bf514edbef7369b7dc33f511

Pyright fails on that sample, which I think is a bug.

I'll need some more time to wrap my head around the pandas example, but in general please feel free to open bugs on typeshed.

@tmke8 tmke8 changed the title Pyright can't find the right type for generic protocol when a concrete type is passed to a generic function BufferedWriter is not compatible with SupportsWrite[AnyStr] Jun 8, 2023
@erictraut erictraut added bug Something isn't working and removed needs investigation Requires additional investigation to determine course of action labels Jun 9, 2023
erictraut pushed a commit that referenced this issue Jun 12, 2023
…pe variables whose values are provided by other argument types in a call. This addresses #5243.
erictraut pushed a commit that referenced this issue Jun 12, 2023
…pe variables whose values are provided by other argument types in a call. This addresses #5243.
erictraut added a commit that referenced this issue Jun 12, 2023
erictraut pushed a commit that referenced this issue Jun 12, 2023
…pe variables whose values are provided by other argument types in a call. This addresses #5243.
erictraut added a commit that referenced this issue Jun 12, 2023
…pe variables whose values are provided by other argument types in a call. This addresses #5243. (#5273)

Co-authored-by: Eric Traut <[email protected]>
erictraut added a commit that referenced this issue Jun 12, 2023
* Improved the protocol matching logic so it honors partially-solved type variables whose values are provided by other argument types in a call. This addresses #5243.

* Updated typeshed to the latest version.

---------

Co-authored-by: Eric Traut <[email protected]>
@erictraut erictraut reopened this Jun 12, 2023
@erictraut
Copy link
Collaborator

This will be fixed in the next release.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Jun 12, 2023
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.314, which I just published. It will also be included in this week's Pylance prerelease build.

@tmke8
Copy link
Author

tmke8 commented Jun 14, 2023

Thanks, the original code snippet works now, but if I set the type of src to typing.Any, then it still fails:

from __future__ import annotations
from io import BufferedWriter
import shutil
from typing import TYPE_CHECKING, Any, AnyStr

if TYPE_CHECKING:
    from _typeshed import SupportsRead, SupportsWrite


def f(s: SupportsRead[AnyStr], t: SupportsWrite[AnyStr]) -> None:
    shutil.copyfileobj(s, t)


def g(s: SupportsRead[bytes], t: SupportsWrite[bytes]) -> None:
    shutil.copyfileobj(s, t)


def h(src: Any, tgt: BufferedWriter) -> None:  # changed: `src` now has type `Any`
    f(src, tgt)  # same error as before
    g(src, tgt)

@erictraut erictraut reopened this Jun 14, 2023
@erictraut erictraut removed the addressed in next version Issue is fixed and will appear in next published version label Jun 14, 2023
erictraut pushed a commit that referenced this issue Jun 14, 2023
…e case where a partially-solved type variable with a solution of `Any` are provided by other argument types in a call. This addresses #5243.
erictraut added a commit that referenced this issue Jun 14, 2023
…e case where a partially-solved type variable with a solution of `Any` are provided by other argument types in a call. This addresses #5243. (#5296)

Co-authored-by: Eric Traut <[email protected]>
@erictraut
Copy link
Collaborator

Thanks for the bug report. This will be addressed in the next release.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Jun 14, 2023
@Hootner

This comment was marked as spam.

@erictraut
Copy link
Collaborator

This is included in pyright 1.1.315, which I just published. It will also be included in this week's insiders release of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants