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

Warn for IO types like io.BytesIO #463

Open
hugovk opened this issue Jan 22, 2024 · 4 comments
Open

Warn for IO types like io.BytesIO #463

hugovk opened this issue Jan 22, 2024 · 4 comments

Comments

@hugovk
Copy link

hugovk commented Jan 22, 2024

No warnings are raised for this:

from io import BytesIO


def open(fp: BytesIO):
    ...

Instead of the concrete io.BytesIO, this should probably be something more specific for what exactly the file-like object actually does, or possibly even typing.BinaryIO.

Similarly for other IO things.

Should flake8-pyi add some warnings in this area?

@srittau
Copy link
Collaborator

srittau commented Jan 22, 2024

In my experience, if a stub is annotated with BytesIO, it really needs a concrete type. I'd also discourage the use of BinaryIO, which – in my opinion – is a legacy type from before protocols existed.

@AlexWaygood
Copy link
Collaborator

There's a large number of IO-related classes in the stdlib, and they all mean subtly different things; this can be pretty confusing. I think it's reasonable to have a lint flagging potential misuse of these, but there's an interesting question of how far we want to go here:

  • We could almost certainly emit a violation whenever io.BytesIO or io.StringIO is used as a parameter annotation. These are concrete classes, and you almost never want to mandate that people have to pass in instances of those specifically
  • Probably we could add TextIOWrapper to that list?
  • What about the many other io-module ABCs -- IOBase, RawIOBase, BufferedIOBase, FileIO, BufferedReader, BufferedWriter, BufferedRandom, BufferedRWPair, TextIOBase? Should we ban all of those in parameter annotations as well, on the basis that you should probably be using a user-defined protocol or, if you really need a broad ABC, a typing.py interface like typing.IO?

@Akuli
Copy link
Collaborator

Akuli commented Jan 22, 2024

Does this solve a practical problem? Has someone actually used a BytesIO argument type, without being aware of the meaning, when working on a project that uses this plugin? It seems like an unlikely combination to me.

@hugovk
Copy link
Author

hugovk commented Jan 22, 2024

🙋

We're in the process of adding types to https://github.com/python-pillow/Pillow and have inadvertently used BytesIO in a number of places. We're not using this plugin yet, but I'll be adding it and fixing its other findings.

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

No branches or pull requests

4 participants