Skip to content

fileinput: Fix TypeVar usage #7934

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

Merged
merged 10 commits into from
May 24, 2022
Merged

fileinput: Fix TypeVar usage #7934

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
860cd4a
fileinput: Fix TypeVar usage
JelleZijlstra May 24, 2022
ed20193
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 24, 2022
c95e8c8
fixes
JelleZijlstra May 24, 2022
3784512
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 24, 2022
1bdf7cd
make it actually covariant
JelleZijlstra May 24, 2022
baeed87
openhook may be None
JelleZijlstra May 24, 2022
ec3cb3f
typing.Protocol
JelleZijlstra May 24, 2022
f27e4a2
U modes
JelleZijlstra May 24, 2022
972f2f4
other way around
JelleZijlstra May 24, 2022
3852e80
Fix lint
AlexWaygood May 24, 2022
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
240 changes: 216 additions & 24 deletions stdlib/fileinput.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import sys
from _typeshed import Self, StrOrBytesPath
from collections.abc import Callable, Iterable, Iterator
from types import TracebackType
from typing import IO, Any, AnyStr, Generic
from typing import IO, Any, AnyStr, Generic, Protocol, TypeVar, overload
from typing_extensions import Literal, TypeAlias

if sys.version_info >= (3, 9):
from types import GenericAlias

__all__ = [
"input",
Expand All @@ -19,40 +23,133 @@ __all__ = [
"hook_encoded",
]

if sys.version_info >= (3, 9):
from types import GenericAlias
if sys.version_info >= (3, 11):
_TextMode: TypeAlias = Literal["r"]
else:
_TextMode: TypeAlias = Literal["r", "rU", "U"]

_AnyStr_co = TypeVar("_AnyStr_co", str, bytes, covariant=True)
Copy link
Member

Choose a reason for hiding this comment

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

We define the same TypeVar (with the same name) in builtins.pyi and os/__init__.pyi — is it worth adding it to _typeshed, do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's simple enough that I don't mind redefining it, but I don't feel strongly.

Copy link
Member

Choose a reason for hiding this comment

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

We can leave it for now


class _HasReadlineAndFileno(Protocol[_AnyStr_co]):
def readline(self) -> _AnyStr_co: ...
def fileno(self) -> int: ...

if sys.version_info >= (3, 10):
# encoding and errors are added
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: _TextMode = ...,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[str]] | None = ...,
encoding: str | None = ...,
errors: str | None = ...,
) -> FileInput[str]: ...
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: str = ...,
openhook: Callable[[StrOrBytesPath, str], IO[AnyStr]] = ...,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
encoding: None = ...,
errors: None = ...,
) -> FileInput[bytes]: ...
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
encoding: str | None = ...,
errors: str | None = ...,
) -> FileInput[AnyStr]: ...
) -> FileInput[Any]: ...

elif sys.version_info >= (3, 8):
# bufsize is dropped and mode and openhook become keyword-only
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: str = ...,
openhook: Callable[[StrOrBytesPath, str], IO[AnyStr]] = ...,
) -> FileInput[AnyStr]: ...
mode: _TextMode = ...,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[str]] | None = ...,
) -> FileInput[str]: ...
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
) -> FileInput[bytes]: ...
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
) -> FileInput[Any]: ...

else:
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
bufsize: int = ...,
mode: _TextMode = ...,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[str]] | None = ...,
) -> FileInput[str]: ...
# Because mode isn't keyword-only here yet, we need two overloads each for
# the bytes case and the fallback case.
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
bufsize: int = ...,
*,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
) -> FileInput[bytes]: ...
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None,
inplace: bool,
backup: str,
bufsize: int,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
) -> FileInput[bytes]: ...
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
bufsize: int = ...,
mode: str = ...,
openhook: Callable[[StrOrBytesPath, str], IO[AnyStr]] = ...,
) -> FileInput[AnyStr]: ...
*,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
) -> FileInput[Any]: ...
@overload
def input(
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None,
inplace: bool,
backup: str,
bufsize: int,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
) -> FileInput[Any]: ...

def close() -> None: ...
def nextfile() -> None: ...
Expand All @@ -65,36 +162,131 @@ def isstdin() -> bool: ...

class FileInput(Iterator[AnyStr], Generic[AnyStr]):
if sys.version_info >= (3, 10):
# encoding and errors are added
@overload
def __init__(
self,
files: None | StrOrBytesPath | Iterable[StrOrBytesPath] = ...,
self: FileInput[str],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: str = ...,
openhook: Callable[[StrOrBytesPath, str], IO[AnyStr]] = ...,
mode: _TextMode = ...,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[str]] | None = ...,
encoding: str | None = ...,
errors: str | None = ...,
) -> None: ...
@overload
def __init__(
self: FileInput[bytes],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
encoding: None = ...,
errors: None = ...,
) -> None: ...
@overload
def __init__(
self: FileInput[Any],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
encoding: str | None = ...,
errors: str | None = ...,
) -> None: ...

elif sys.version_info >= (3, 8):
# bufsize is dropped and mode and openhook become keyword-only
@overload
def __init__(
self: FileInput[str],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: _TextMode = ...,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[str]] | None = ...,
) -> None: ...
@overload
def __init__(
self: FileInput[bytes],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
) -> None: ...
@overload
def __init__(
self,
files: None | StrOrBytesPath | Iterable[StrOrBytesPath] = ...,
self: FileInput[Any],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
*,
mode: str = ...,
openhook: Callable[[StrOrBytesPath, str], IO[AnyStr]] = ...,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
) -> None: ...

else:
@overload
def __init__(
self,
files: None | StrOrBytesPath | Iterable[StrOrBytesPath] = ...,
self: FileInput[str],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
bufsize: int = ...,
mode: str = ...,
openhook: Callable[[StrOrBytesPath, str], IO[AnyStr]] = ...,
mode: _TextMode = ...,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[str]] | None = ...,
) -> None: ...
# Because mode isn't keyword-only here yet, we need two overloads each for
# the bytes case and the fallback case.
Comment on lines +247 to +248
Copy link
Member

Choose a reason for hiding this comment

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

This is annoying, but it doesn't look like there's any other way of doing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sebastian opened a conversation a while ago about allowing arguments without defaults to follow arguments with defaults, which would make this easier, but it didn't go anywhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There was some pushback, because that's mostly useful for typing and considered bad API design otherwise, but honestly more from typing-critical developers, if I remember correctly.

Copy link
Member

Choose a reason for hiding this comment

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

Plus ça change, plus c'est la même chose

Copy link
Member Author

Choose a reason for hiding this comment

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

An alternative could be to add a special marker like typing.NO_DEFAULT, which we'd use as the default. That would look like this:

        @overload
        def __init__(
            self: FileInput[bytes],
            files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
            inplace: bool = ...,
            backup: str = ...,
            bufsize: int = ...,
            mode: Literal["rb"] = NO_DEFAULT,
            openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
        ) -> None: ...

And the semantics would be that type checkers don't pick the overload if no value is given for mode, but it's still the 6th positional argument.

Copy link
Member

Choose a reason for hiding this comment

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

An alternative could be to add a special marker like typing.NO_DEFAULT, which we'd use as the default.

Oh, I like that quite a lot, actually. It would mean we wouldn't have to change Python's grammar at all. It does also feel like the kind of thing that would disproportionately benefit typeshed, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

There was also a recent mypy issue where end-users were complaining about this, so it might be more broadly useful. If @srittau likes it too I can send it to typing-sig.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be possible to write a script that automatically applies NO_DEFAULT in typeshed wherever possible. I would be interested to see the results,

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was the thread. Looking back at it, the push back wasn't too bad. Maybe it would be worth trying again with a complete PEP?

I'm not particularly fond of typing.NO_DEFAULT, but would prefer it over the status quo.

Copy link
Member

@AlexWaygood AlexWaygood May 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think I actually prefer typing.NO_DEFAULT. The status quo is definitely annoying for type annotations, but I think syntax changes should have to meet a pretty high bar, and I'm not sure this meets that bar ://

typing.NO_DEDAULT feels like it would be much simpler to implement with minimal changes to the language itself (and it would also be much easier to backport).

@overload
def __init__(
self: FileInput[bytes],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
bufsize: int = ...,
*,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
) -> None: ...
@overload
def __init__(
self: FileInput[bytes],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None,
inplace: bool,
backup: str,
bufsize: int,
mode: Literal["rb"],
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[bytes]] | None = ...,
) -> None: ...
@overload
def __init__(
self: FileInput[Any],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None = ...,
inplace: bool = ...,
backup: str = ...,
bufsize: int = ...,
*,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
) -> None: ...
@overload
def __init__(
self: FileInput[Any],
files: StrOrBytesPath | Iterable[StrOrBytesPath] | None,
inplace: bool,
backup: str,
bufsize: int,
mode: str,
openhook: Callable[[StrOrBytesPath, str], _HasReadlineAndFileno[Any]] | None = ...,
) -> None: ...

def __del__(self) -> None: ...
Expand Down