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

socket.makefile assumes wrong overloaded form #3977

Closed
jnsnow opened this issue May 12, 2020 · 1 comment · Fixed by #3978
Closed

socket.makefile assumes wrong overloaded form #3977

jnsnow opened this issue May 12, 2020 · 1 comment · Fixed by #3978

Comments

@jnsnow
Copy link
Contributor

jnsnow commented May 12, 2020

Hello, I'm not sure if this is a typeshed issue or a mypy issue. I don't know how to diagnose the difference. I am leaning on the idea that it is a typeshed issue. (I also don't know how to tell which version of typeshed mypy is using, but I am using mypy 0.770 as installed from pip3 in a virtual environment.)

In QEMU, we have some code like this:

    def connect(self) -> None:
        self._sock.connect(self._address)
        self._sockfile = self._sock.makefile()

It's my understanding that this will create a text-mode socket by default. In practice, this does appear to be the case, so we have a function like this:

    def cmd(self, qtest_cmd: str) -> str:
        assert self._sockfile is not None
        self._sock.sendall((qtest_cmd + "\n").encode('utf-8'))
        resp = self._sockfile.readline()
        return resp

mypy, however, does not like this because it believes makefile to return a BinaryIO type, but we have annotated the socket makefile to be Optional[TextIO].

Looking to typeshed, https://github.com/python/typeshed/blob/master/stdlib/2and3/socket.pyi#L640
I find that the function is overloaded with regards to a mode of '' to imply the TextIO variant, but it's not clear to me what should happen if the mode argument is omitted entirely, and isn't an empty string.

In practice, mypy believes that makefile() returns a BinaryIO when it appears to return a TextIO kind. (In reality, a _io.TextIOWrapper)

I CAN work around this by appending mode="r" to the arguments, but I wasn't sure if this is a mypy bug or a typeshed bug, but I wanted to try and report it to see if I could understand the problem better.

hauntsaninja pushed a commit to hauntsaninja/typeshed that referenced this issue May 12, 2020
@hauntsaninja
Copy link
Collaborator

Thanks for reporting! Looks like this is a bug in typeshed. I've attached a PR; the issue is that the overload with the default value for mode should have been the one that returned TextIO, since the default mode is "r".

JelleZijlstra pushed a commit that referenced this issue May 13, 2020
Fixes #3977

Co-authored-by: hauntsaninja <>
philmd pushed a commit to philmd/qemu that referenced this issue May 31, 2020
Note:

A bug in typeshed (python/typeshed#3977)
misinterprets the type of makefile(). Work around this by explicitly
stating that we are opening a text-mode file.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20200514055403.18902-13-jsnow@redhat.com>
qemu-deploy pushed a commit to qemu/qemu that referenced this issue Jun 1, 2020
Note:

A bug in typeshed (python/typeshed#3977)
misinterprets the type of makefile(). Work around this by explicitly
stating that we are opening a text-mode file.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200514055403.18902-13-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
vishalkuo pushed a commit to vishalkuo/typeshed that referenced this issue Jun 26, 2020
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 a pull request may close this issue.

2 participants