make Buffer compatible with python 3.10 and above #144
Conversation
Aiqin-Aiven
force-pushed
the
aiqin-rr
branch
from
30242fe to
834d738
Compare
| @@ -205,10 +213,10 @@ def seek(self, offset: int, whence: int = 0) -> int: | |||
| def truncate(self, size: Optional[int] = None) -> int: | |||
| raise UnsupportedOperation("truncate") | |||
|
|
|||
| def write(self, s: Union[bytes, Buffer]) -> int: | |||
| def write(self, s: Union[bytes, Buffer]) -> int: # type: ignore | |||
There was a problem hiding this comment.
Why do you need to ignore the type here (and in writelines)?
There was a problem hiding this comment.
BinaryIO wants only bytes and no Buffer as input for write since it's basically just IO[bytes], but I believe in some places we actually do want to forward some buffer instead of bytes hence why Buffer appears here.
I believe that currently there is no way to avoid ignoring some cases because we have some dependencies that want BinaryIO and other places that don't want that in their typing. So if we remove Buffer here and make this a "proper" BinaryIO then you'll find similar comments when we use this class instead.
There was a problem hiding this comment.
I believe we don't have other better option here, right?
There was a problem hiding this comment.
Yeah I don't think we have better options now. Unfortunately this area of typing is a bit of a mess...
There was a problem hiding this comment.
The comment is not blocking from my side, was just trying to understand the issue. TY @giacomo-alzetta-aiven for breaking it down. The only concern was that it might hide legit issues that can pop up in the future. Maybe even that is overblown? I don't know.
Aiqin-Aiven
force-pushed
the
aiqin-rr
branch
from
834d738 to
585a367
Compare
Codecov Report
@@ Coverage Diff @@
## main #144 +/- ##
==========================================
+ Coverage 66.38% 66.42% +0.04%
==========================================
Files 35 35
Lines 3879 3884 +5
==========================================
+ Hits 2575 2580 +5
Misses 1304 1304
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Aiqin-Aiven
force-pushed
the
aiqin-rr
branch
from
585a367 to
647ad9f
Compare
There was a problem hiding this comment.
Everything is much easier:
Bufferwas introduced totyping_extensionsin4.6.0: https://github.com/python/typing_extensions/blob/main/CHANGELOG.md#release-460-may-22-2023pghoardpinstyping_extensions==4.5.0: https://github.com/Aiven-Open/pghoard/blob/21bc21446977cadd2ac42e967bbba846d930f9d5/constraints.txt#L78typing_extensions.Buffersupports all Python versions: https://github.com/python/typing_extensions/blob/c17c499b865585458bc334a1f895ebaedd2ab854/src/typing_extensions.py#L2522-L2548- So, you need just bump
typing_extensionsinpghoardto at least4.6.0. I would bump it to4.7.1, since it's what we currently have downstream.
|
As per: Aiven-Open/pghoard#601 |
Fix the error on pghoard pipeline for python 3.10 and above
E ImportError: cannot import name 'Buffer' from 'typing_extensions' (/opt/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/typing_extensions.py)
Why this way
In Python 3.10 and later, the
Buffername is no longer provided bytyping_extensions. Instead, you can usetyping.ByteStringortyping.MutableSequence[bytes]as a replacement forBuffer.