-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Unify file descriptor definitions #3584
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
stdlib/2and3/_types.pyi
Outdated
@@ -0,0 +1,7 @@ | |||
from typing import Protocol, Union | |||
|
|||
class _HasFileno(Protocol): |
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.
Not sure it's necessary to give everything in this file a leading underscore. However, we should use the @type_check_only
decorator.
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.
When I added @type_check_only
I got a bunch of errors from pytype, any idea what caused them?
Ran pytype with 1754 pyis, got 30 errors.
stdlib/2/fcntl.pyi: ParseError: syntax error, unexpected CLASS, expecting DEF
stdlib/2and3/_types.pyi: ParseError: syntax error, unexpected CLASS, expecting DEF
stdlib/2and3/_types.pyi: ParseError: syntax error, unexpected CLASS, expecting DEF
stdlib/2and3/asynchat.pyi: ParseError: syntax error, unexpected CLASS, expecting DEF
...
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.
pytype's parser is a little on the strict side sometimes. @rchen152 might be able to help.
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's a specific set of decorators that the pytype parser allows - looks like we never added @type_check_only
to it. I'll go ahead and do that.
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.
Ok, I'll add it to HasFileno and mark this resolved.
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've fixed the dev version of pytype - once our nightly tests run, I'll know whether we can do a pytype release tomorrow or if it'll take a few days. Either way, I'll update here.
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 pushed the new pytype version. I reran the pytype test, but it looks like it's still downloading the old version. I'll wait a bit and try the test again, and merge the PR if it passes.
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.
Very nice cleanup. Just one comment below.
The _types module can house any common type defintions used throughout the rest of typeshed to keep defintions in sync. First candidate is file descriptors where anything with `fileno()` method is accepted. There were several different implementations in various files that can be unified.
Since pytype doesn't do anything with this decorator, we parse and then discard it. The decorator is allowed on both classes and functions (https://docs.python.org/3/library/typing.html#typing.type_check_only). For python/typeshed#3584. PiperOrigin-RevId: 288569410
The _types module can house any common type defintions used throughout
the rest of typeshed to keep defintions in sync.
First candidate is "file[-like] objects" where anything with
fileno()
method is accepted. There were several different implementations in
various files that can be unified.
(Continued from #3582, changed branch names)