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.

Sign up for GitHub

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

make Buffer compatible with python 3.10 and above #144

Closed
wants to merge 1 commit into from
Closed

make Buffer compatible with python 3.10 and above #144

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions rohmu/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,15 @@
from itertools import islice
from rohmu.typing import HasFileno
from typing import BinaryIO, Generator, Iterable, Optional, Tuple, TypeVar, Union
from typing_extensions import Buffer

import sys

if sys.version_info < (3, 10):
from typing_extensions import Buffer
else:
from typing import ByteString, MutableSequence

Buffer = Union[ByteString, MutableSequence[bytes]]

import fcntl
import logging
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to ignore the type here (and in writelines)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we don't have other better option here, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I don't think we have better options now. Unfortunately this area of typing is a bit of a mess...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

raise UnsupportedOperation("write")

def writelines(self, __lines: Union[Iterable[bytes], Iterable[Buffer]]) -> None:
def writelines(self, __lines: Union[Iterable[bytes], Iterable[Buffer]]) -> None: # type: ignore
raise UnsupportedOperation("writelines")

def fileno(self) -> int:
Expand Down
Loading