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

Allow memoryview[bytes] in write() and writelines() #13519

Merged
merged 18 commits into from
Mar 20, 2025

Conversation

Dreamsorcerer
Copy link
Contributor

We have tests on aiohttp which end up passing memoryview[bytes] to these methods and they seem to work. With mypy's --strict-bytes it gives a type error due to the missing type in typeshed.

@Dreamsorcerer
Copy link
Contributor Author

pytype refuses to allow specifying the type parameter...

This comment has been minimized.

@JelleZijlstra
Copy link
Member

Should this be ReadableBuffer instead?

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@Dreamsorcerer
Copy link
Contributor Author

Should this be ReadableBuffer instead?

Seems to work, but as python-chess highlights, it will require any subclasses to update their annotations.

This comment has been minimized.

This comment has been minimized.

Co-authored-by: Sebastian Rittau <[email protected]>
@srittau
Copy link
Collaborator

srittau commented Mar 20, 2025

I'm still unsure what problem pytype has. I'm also not sure whom to ping about this nowadays. Adding the two changes files to tests/pytype_exclude_list.txt might help.

This comment has been minimized.

Copy link
Contributor

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

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/http_writer.py:101: error: Unused "type: ignore" comment  [unused-ignore]
+ aiohttp/http_writer.py:120: error: Unused "type: ignore" comment  [unused-ignore]

@srittau srittau merged commit b4e49dd into python:main Mar 20, 2025
63 checks passed
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.

3 participants