-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
Conversation
Fixes python#7922, part of python#7928. Also discovered an apparent CPython bug in the process: python/cpython#93157
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs say:
With mode you can specify which file mode will be passed to open(). It must be one of 'r', 'rU', 'U' and 'rb'.
https://docs.python.org/3/library/fileinput.html#fileinput.FileInput
But 'rU' and 'U' have been deprecated since Python 3.4, so perhaps it doesn't matter
Good catch! U support was removed in 3.11 (python/cpython@19ba212) and I didn't check the earlier branches carefully enough. I'll add it. |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
else: | ||
_TextMode: TypeAlias = Literal["r", "rU", "U"] | ||
|
||
_AnyStr_co = TypeVar("_AnyStr_co", str, bytes, covariant=True) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
# Because mode isn't keyword-only here yet, we need two overloads each for | ||
# the bytes case and the fallback case. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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,
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
Fixes #7922, part of #7928.
Also discovered an apparent CPython bug in the process: python/cpython#93157