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

Type stub incompatiblity with tarfile.ExFileObject (i.e. tarfile.TarFile.extractfile() result) #214

Closed
mgorny opened this issue Nov 9, 2022 · 6 comments · Fixed by #215 or hukkin/tomli-w#41

Comments

@mgorny
Copy link

mgorny commented Nov 9, 2022

Please consider the following snippet:

import tomli
import tarfile

with tarfile.open("test.tar") as tar:
    f = tar.extractfile("test.toml")
    assert f is not None
    tomli.load(f)

When I run mypy on it, I get:

test.py:7: error: Argument 1 to "load" has incompatible type "IO[bytes]"; expected "BinaryIO"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

To be honest, typing docs don't really say what the difference between IO[bytes] and BinaryIO is. Besides type stubs, the code works. FWICS, tomllib would suffer from the same problem if not for the fact that mypy overrides its type stub to:

def load(__fp: SupportsRead[bytes], *, parse_float: Callable[[str], Any] = ...) -> dict[str, Any]: ...
@hukkin
Copy link
Owner

hukkin commented Nov 9, 2022

Thanks for bringing this up!

I'd like to hear what @hauntsaninja has to say here, as they are far more knowledgeable in Python typing than I am.

It seems to me that BinaryIO should be pretty much equivalent to IO[bytes] (even the __enter__ method override seems redundant to me, but there must be a good reason for it). But it is a subtype of IO, hence the incompatibility (I think).

Perhaps tarfile should move to using BinaryIO instead of IO[bytes] or mypy should not error?

We can consider moving to using a custom SupportsRead[bytes] here instead of BinaryIO, which is too strict a requirement anyways but was convenient to use because it's in the stdlib.

@hauntsaninja
Copy link
Collaborator

I think using something like SupportsRead[bytes] might end up being the most user friendly thing.

I think BinaryIO should be avoided. There's not really any difference between it and IO[bytes] and it mainly just serves to confuse. In general, typeshed tries to avoid use of BinaryIO and TextIO, and even of IO. These things were added to the stdlib before protocols were a thing. In practice they often don't work well because it turns out there's a tonne of variation in what "file-like" actually means, and they don't capture well the difference between readers and writers. There's more discussion at python/typing#829

@mgorny
Copy link
Author

mgorny commented Nov 10, 2022

Thanks for the fix. Would you also submit it to tomllib or should I try?

@hauntsaninja
Copy link
Collaborator

We don't need to change tomllib, since nothing looks at type hints in the standard library; everything just looks at typeshed

@hukkin
Copy link
Owner

hukkin commented Nov 10, 2022

Yup, and typeshed already uses SupportsRead so you shouldn't have this issue with tomllib.

@mgorny
Copy link
Author

mgorny commented Nov 10, 2022

We don't need to change tomllib, since nothing looks at type hints in the standard library; everything just looks at typeshed

Well, I did look at types in the stdlib at first and took me quite a while to figure out why mypy complains about tomli and not the "identical" types in tomllib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants