| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| Expand Up | @@ -46,12 +46,14 @@ | |||||
| "BufferedReader", "BufferedWriter", "BufferedRWPair", | ||||||
| "BufferedRandom", "TextIOBase", "TextIOWrapper", | ||||||
| "UnsupportedOperation", "SEEK_SET", "SEEK_CUR", "SEEK_END", | ||||||
| "DEFAULT_BUFFER_SIZE", "text_encoding", "IncrementalNewlineDecoder"] | ||||||
| "DEFAULT_BUFFER_SIZE", "text_encoding", "IncrementalNewlineDecoder", | ||||||
| "Reader", "Writer"] | ||||||
|
|
||||||
|
|
||||||
| import _io | ||||||
| import abc | ||||||
|
|
||||||
| from _collections_abc import _check_methods | ||||||
| from _io import (DEFAULT_BUFFER_SIZE, BlockingIOError, UnsupportedOperation, | ||||||
| open, open_code, FileIO, BytesIO, StringIO, BufferedReader, | ||||||
| BufferedWriter, BufferedRWPair, BufferedRandom, | ||||||
| Expand Down Expand Up | @@ -97,3 +99,55 @@ class TextIOBase(_io._TextIOBase, IOBase): | |||||
| pass | ||||||
| else: | ||||||
| RawIOBase.register(_WindowsConsoleIO) | ||||||
|
|
||||||
| # | ||||||
| # Static Typing Support | ||||||
| # | ||||||
|
|
||||||
| GenericAlias = type(list[int]) | ||||||
|
|
||||||
|
|
||||||
| class Reader(metaclass=abc.ABCMeta): | ||||||
| """Protocol for simple I/O reader instances. | ||||||
|
|
||||||
| This protocol only supports blocking I/O. | ||||||
| """ | ||||||
|
|
||||||
| __slots__ = () | ||||||
|
|
||||||
| @abc.abstractmethod | ||||||
| def read(self, size=..., /): | ||||||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualitySorry if this has been discussed before, but I'm unsure on the runtime use of size=... (I didn't notice this earlier in my documentation review, sorry). Almost every other read(size) method I can find has a default of either None or -1. I also can't find another method in the stdlib with a default of ... (outside of the recently-added protocols in wsgiref.types). Would it be better to have size=-1, to indicate that the method takes an int? I'm not sure how much we want typeshed-like practices to leak into the standard library.
Suggested change
A
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityMandating defaults is not really something you can do in a protocol. I also wouldn't want to mandate that implementors have to use a default of -1, because – as you said – some implementations use None.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityRight, but this isn't a protocol -- it's an ABC, which do have defaults -- see e.g. collections.abc.Generator. All of the read() methods in io are documented as having read(size=-1, /), and given these ABCs are going into io, I think we should be consistent with that interface, or have good reason to diverge from it (& document why).
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIt's supposed to be a protocol, not an ABC. (Notwithstanding the fact that all protocols are ABCs.) It's just a protocol in the implementation for performance reasons. And using -1 as a default would give users the very wrong impression that they can use read(-1) when that may or may not actually be supported.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
From the documentation of io.RawIOBase.read():
I would expect the io.Reader.read() ABC/protocol to have this same guarantee, for a 'properly' implemented read() method (according to the io expecations). In the proposed documentation, we say:
This forbids None (good!), but is silent on what happens should size be omitted. I still think we should use -1 instead of ..., but at the very least we should include in the documentation the contract for what happens when read() is called with no arguments.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI disagree. -1 is the default for some implementations, but the protocol should not make any default mandatory.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI'm still not sure I follow, though -- currently the ABC/protocol has the mandatory default of size=..., which is always invalid and not the correct type. -1 is valid for all interfaces specified by the io documentation, which is where this new type is being added. I ran the following with mypy --strict and it passed, so I don't think type checkers care about default values (and as discussed, the type forbids using non-integer types): import abc
class Reader(metaclass=abc.ABCMeta):
__slots__ = ()
@abc.abstractmethod
def read(self, size: int = -1, /) -> bytes: pass
class CustomReader(Reader):
def read(self, size: int = -1, /) -> bytes:
return b''
class CustomReaderZero(Reader):
def read(self, size: int = 0, /) -> bytes:
return b''
assert issubclass(CustomReader, Reader)
assert issubclass(CustomReaderZero, Reader)
assert isinstance(CustomReader(), Reader)
assert isinstance(CustomReaderZero(), Reader)
def reader_func(r: Reader) -> None:
r.read()
reader_func(CustomReader())
reader_func(CustomReaderZero())
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI agree with Sebastian here; we should use ... because the protocol need not mandate any particular default.
The protocol should also match other file-like classes defined elsewhere in the stdlib or even in third-party libraries. When defining a protocol it's often useful to be permissive, so that all objects that are intended to match the protocol actually match it.
Sorry, something went wrong.
All reactions
|
||||||
| """Read data from the input stream and return it. | ||||||
|
|
||||||
| If *size* is specified, at most *size* items (bytes/characters) will be | ||||||
| read. | ||||||
| """ | ||||||
|
|
||||||
| @classmethod | ||||||
| def __subclasshook__(cls, C): | ||||||
| if cls is Reader: | ||||||
| return _check_methods(C, "read") | ||||||
| return NotImplemented | ||||||
|
|
||||||
| __class_getitem__ = classmethod(GenericAlias) | ||||||
|
|
||||||
|
|
||||||
| class Writer(metaclass=abc.ABCMeta): | ||||||
| """Protocol for simple I/O writer instances. | ||||||
|
|
||||||
| This protocol only supports blocking I/O. | ||||||
| """ | ||||||
|
|
||||||
| __slots__ = () | ||||||
|
|
||||||
| @abc.abstractmethod | ||||||
| def write(self, data, /): | ||||||
| """Write *data* to the output stream and return the number of items written.""" | ||||||
|
|
||||||
| @classmethod | ||||||
| def __subclasshook__(cls, C): | ||||||
| if cls is Writer: | ||||||
| return _check_methods(C, "write") | ||||||
| return NotImplemented | ||||||
|
|
||||||
| __class_getitem__ = classmethod(GenericAlias) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Add protocols :class:`io.Reader` and :class:`io.Writer` as | ||
| alternatives to :class:`typing.IO`, :class:`typing.TextIO`, and | ||
| :class:`typing.BinaryIO`. |
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.