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

re-sort io classes into _io #12755

Merged
merged 3 commits into from
Oct 9, 2024
Merged

re-sort io classes into _io #12755

merged 3 commits into from
Oct 9, 2024

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Oct 8, 2024

This version keeps it simple and clean: No changes to class bodies. The only changes here are moving between files and updating the naming and inheritance.

Related to #3968 and split from #12740.

tungol added 3 commits October 8, 2024 11:06
This version keeps it simple and clean: No changes to class bodies.
The only changes here are moving between files and updating the
naming and inheritance.

Related to python#3968 and split from python#12740.
@tungol
Copy link
Contributor Author

tungol commented Oct 8, 2024

For convenience of review, here is the complete diff from the version of io.py prior to this MR to the version of _io.py in this MR:

--- io.py (old version)
+++ _io.py (new version)
@@ -1,56 +1,25 @@
-import abc
 import builtins
 import codecs
 import sys
 from _typeshed import FileDescriptorOrPath, MaybeNone, ReadableBuffer, WriteableBuffer
 from collections.abc import Callable, Iterable, Iterator
+from io import BufferedIOBase, RawIOBase, TextIOBase, UnsupportedOperation as UnsupportedOperation
 from os import _Opener
 from types import TracebackType
 from typing import IO, Any, BinaryIO, Final, Generic, Literal, Protocol, TextIO, TypeVar, overload, type_check_only
 from typing_extensions import Self
 
-__all__ = [
-    "BlockingIOError",
-    "open",
-    "open_code",
-    "IOBase",
-    "RawIOBase",
-    "FileIO",
-    "BytesIO",
-    "StringIO",
-    "BufferedIOBase",
-    "BufferedReader",
-    "BufferedWriter",
-    "BufferedRWPair",
-    "BufferedRandom",
-    "TextIOBase",
-    "TextIOWrapper",
-    "UnsupportedOperation",
-    "SEEK_SET",
-    "SEEK_CUR",
-    "SEEK_END",
-]
-
-if sys.version_info >= (3, 11):
-    __all__ += ["DEFAULT_BUFFER_SIZE", "IncrementalNewlineDecoder", "text_encoding"]
-
 _T = TypeVar("_T")
 
 DEFAULT_BUFFER_SIZE: Final = 8192
 
-SEEK_SET: Final = 0
-SEEK_CUR: Final = 1
-SEEK_END: Final = 2
-
 open = builtins.open
 
 def open_code(path: str) -> IO[bytes]: ...
 
 BlockingIOError = builtins.BlockingIOError
 
-class UnsupportedOperation(OSError, ValueError): ...
-
-class IOBase(metaclass=abc.ABCMeta):
+class _IOBase:
     def __iter__(self) -> Iterator[bytes]: ...
     def __next__(self) -> bytes: ...
     def __enter__(self) -> Self: ...
@@ -77,7 +46,7 @@
     def closed(self) -> bool: ...
     def _checkClosed(self) -> None: ...  # undocumented
 
-class RawIOBase(IOBase):
+class _RawIOBase(_IOBase):
     def readall(self) -> bytes: ...
     # The following methods can return None if the file is in non-blocking mode
     # and no data is available.
@@ -85,7 +54,7 @@
     def write(self, b: ReadableBuffer, /) -> int | MaybeNone: ...
     def read(self, size: int = -1, /) -> bytes | MaybeNone: ...
 
-class BufferedIOBase(IOBase):
+class _BufferedIOBase(_IOBase):
     def detach(self) -> RawIOBase: ...
     def readinto(self, buffer: WriteableBuffer, /) -> int: ...
     def write(self, buffer: ReadableBuffer, /) -> int: ...
@@ -93,7 +62,7 @@
     def read(self, size: int | None = ..., /) -> bytes: ...
     def read1(self, size: int = ..., /) -> bytes: ...
 
-class FileIO(RawIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of writelines in the base classes
+class FileIO(RawIOBase, _RawIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of writelines in the base classes
     mode: str
     # The type of "name" equals the argument passed in to the constructor,
     # but that can make FileIO incompatible with other I/O types that assume
@@ -106,7 +75,7 @@
     def closefd(self) -> bool: ...
     def __enter__(self) -> Self: ...
 
-class BytesIO(BufferedIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of methods in the base classes
+class BytesIO(BufferedIOBase, _BufferedIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of methods in the base classes
     def __init__(self, initial_bytes: ReadableBuffer = ...) -> None: ...
     # BytesIO does not contain a "name" field. This workaround is necessary
     # to allow BytesIO sub-classes to add this field, as it is defined
@@ -117,27 +86,27 @@
     def getbuffer(self) -> memoryview: ...
     def read1(self, size: int | None = -1, /) -> bytes: ...
 
-class BufferedReader(BufferedIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of methods in the base classes
+class BufferedReader(BufferedIOBase, _BufferedIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of methods in the base classes
     raw: RawIOBase
     def __enter__(self) -> Self: ...
     def __init__(self, raw: RawIOBase, buffer_size: int = ...) -> None: ...
     def peek(self, size: int = 0, /) -> bytes: ...
 
-class BufferedWriter(BufferedIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of writelines in the base classes
+class BufferedWriter(BufferedIOBase, _BufferedIOBase, BinaryIO):  # type: ignore[misc]  # incompatible definitions of writelines in the base classes
     raw: RawIOBase
     def __enter__(self) -> Self: ...
     def __init__(self, raw: RawIOBase, buffer_size: int = ...) -> None: ...
     def write(self, buffer: ReadableBuffer, /) -> int: ...
 
-class BufferedRandom(BufferedReader, BufferedWriter):  # type: ignore[misc]  # incompatible definitions of methods in the base classes
+class BufferedRandom(BufferedReader, BufferedWriter, BufferedIOBase, _BufferedIOBase):  # type: ignore[misc]  # incompatible definitions of methods in the base classes
     def __enter__(self) -> Self: ...
     def seek(self, target: int, whence: int = 0, /) -> int: ...  # stubtest needs this
 
-class BufferedRWPair(BufferedIOBase):
+class BufferedRWPair(BufferedIOBase, _BufferedIOBase):
     def __init__(self, reader: RawIOBase, writer: RawIOBase, buffer_size: int = ...) -> None: ...
     def peek(self, size: int = ..., /) -> bytes: ...
 
-class TextIOBase(IOBase):
+class _TextIOBase(_IOBase):
     encoding: str
     errors: str | None
     newlines: str | tuple[str, ...] | None
@@ -176,7 +145,7 @@
 
 _BufferT_co = TypeVar("_BufferT_co", bound=_WrappedBuffer, default=_WrappedBuffer, covariant=True)
 
-class TextIOWrapper(TextIOBase, TextIO, Generic[_BufferT_co]):  # type: ignore[misc]  # incompatible definitions of write in the base classes
+class TextIOWrapper(TextIOBase, _TextIOBase, TextIO, Generic[_BufferT_co]):  # type: ignore[misc]  # incompatible definitions of write in the base classes
     def __init__(
         self,
         buffer: _BufferT_co,
@@ -217,7 +186,7 @@
     # operations.
     def seek(self, cookie: int, whence: int = 0, /) -> int: ...
 
-class StringIO(TextIOWrapper):
+class StringIO(TextIOWrapper, TextIOBase, _TextIOBase):  # type: ignore[misc]  # incompatible definitions of write in the base classes
     def __init__(self, initial_value: str | None = ..., newline: str | None = ...) -> None: ...
     # StringIO does not contain a "name" field. This workaround is necessary
     # to allow StringIO sub-classes to add this field, as it is defined

Copy link
Contributor

github-actions bot commented Oct 8, 2024

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

scrapy (https://github.com/scrapy/scrapy)
+ scrapy/core/downloader/handlers/ftp.py:61: error: Incompatible types in assignment (expression has type "BufferedIOBase", variable has type "BinaryIO")  [assignment]

operator (https://github.com/canonical/operator)
- ops/pebble.py:1878: error: Argument 1 of "write" is incompatible with supertype "BufferedIOBase"; supertype defines the argument type as "Buffer"  [override]
+ ops/pebble.py:1878: error: Argument 1 of "write" is incompatible with supertype "_BufferedIOBase"; supertype defines the argument type as "Buffer"  [override]
- ops/pebble.py:1902: error: Return type "str | bytes" of "read" incompatible with return type "bytes" in supertype "BufferedIOBase"  [override]
+ ops/pebble.py:1902: error: Return type "str | bytes" of "read" incompatible with return type "bytes" in supertype "_BufferedIOBase"  [override]
- ops/pebble.py:1902: error: Argument 1 of "read" is incompatible with supertype "BufferedIOBase"; supertype defines the argument type as "int | None"  [override]
+ ops/pebble.py:1902: error: Argument 1 of "read" is incompatible with supertype "_BufferedIOBase"; supertype defines the argument type as "int | None"  [override]
- ops/pebble.py:1935: error: Return type "str | bytes" of "read1" incompatible with return type "bytes" in supertype "BufferedIOBase"  [override]
+ ops/pebble.py:1935: error: Return type "str | bytes" of "read1" incompatible with return type "bytes" in supertype "_BufferedIOBase"  [override]

paasta (https://github.com/yelp/paasta)
- paasta_tools/cli/cmds/local_run.py:980: error: Argument 1 to "write" of "TextIOBase" has incompatible type "Union[str, bytes]"; expected "str"  [arg-type]
+ paasta_tools/cli/cmds/local_run.py:980: error: Argument 1 to "write" of "_TextIOBase" has incompatible type "Union[str, bytes]"; expected "str"  [arg-type]

@tungol
Copy link
Contributor Author

tungol commented Oct 8, 2024

I mentioned this in the previous version of this MR, but worth noting again: I've chosen to keep explicit bases explicit. For example in class BufferedRWPair(BufferedIOBase, _BufferedIOBase), io.BufferedIOBase already inherits from _io._BufferedIOBase, but _io.BufferedRWPair actually only inherits from _io._BufferedIOBase at runtime while io.BufferedIOBase is the ABC that it gets registered to.

There was no technical requirement for keeping _io._BufferedIOBase as an explicit base of BufferedRWPair, but doing so is a small reminder of the runtime implementation and also means that stubtest inheritance check will show that typeshed is adding bases without removing any, and I think that's cleaner.

StringIO(TextIOWrapper, TextIOBase, _TextIOBase) is slightly an odd one out by that rule. Runtime inherits from _TextIOBase, and is ABC-registered to TextIOBase. TextIOWrapper also inherits from _TextIOBase and is ABC-registered to TextIOBase, so that inheritance for StringIO is doubly redundant. However: Runtime StringIO neither inherits nor is it ABC-registered to TextIOWrapper. The python implementation of StringIO in _pyio does inherit from TextIOWrapper, so there's an intention there. I guess I'm on the fence of whether that means that StringIO(TextIOWrapper, TextIOBase, _TextIOBase) or StringIO(TextIOWrapper, _TextIOBase) is more reasonable, but the former fits the pattern better so that's what I went with.

_io.BufferedWriter.seek
_io.BufferedWriter.truncate
_io.BytesIO.readlines
_io.BytesIO.seek # Parameter name for a positional-only param differs from its name in the inherited method
Copy link
Member

Choose a reason for hiding this comment

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

This sounds like something we can fix; mypy shouldn't complain about name mismatches in pos-only params.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, but just to note that the new allow-list entries are just copies of the existing io.* allow-list entries, updated for _io.*

@JelleZijlstra JelleZijlstra merged commit 2b1c7d5 into python:main Oct 9, 2024
63 checks passed
@tungol tungol deleted the io-move branch October 9, 2024 04:39
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.

2 participants