-
-
Notifications
You must be signed in to change notification settings - Fork 2k
clean python 3 specific stuff from stdlib/@python2 stubs #5451
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
This reverts commit baa5dbe.
|
Script breaks edit: done |
JelleZijlstra
left a comment
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.
Only read as far as array
| ReadableBuffer = Union[bytes, bytearray, memoryview, array.array[Any], mmap.mmap, buffer] | ||
| WriteableBuffer = Union[bytearray, memoryview, array.array[Any], mmap.mmap, buffer] | ||
|
|
||
| # Used by type checkers for checks involving None (does not exist at runtime) |
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.
| # Used by type checkers for checks involving None (does not exist at runtime) |
It does exist in Python 2
$ python2
Python 2.7.12 (default, Nov 12 2018, 14:36:49)
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import types
>>> types.NoneType
<type 'NoneType'>
>>>
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.
Yes, types.NoneType exists, but the comment is about _typeshed.NoneType.
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.
My reading comprehension needs some work sometimes :)
In that case the comment still doesn't make much sense, since none of this file exists at runtime.
| @@ -1,4 +1 @@ | |||
| import sys | |||
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 guess you'll have to remove the import too.
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 Python 2 stubs don't get checked for missing imports. We have this in .flake8:
exclude = .venv*,@*,.git,*_pb2.pyi
Is this intentional?
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 what that's there for. It predates the @python2 directory, since it was there before 9af49c0. I think we can just remove it.
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.
Looking at git history, this is not intentional; ignoring @* was added before the big directory restructuring.
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 should probably remove it after this PR, since I expect many new errors.
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 started cleaning unused imports from Python 2 stubs, but I'd rather have that in a separate PR because it's a lot of manual work.
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.
Sounds good. I think we should just merge this PR and follow up on other issues later.
#5442 copied files
Done with quick and dirty script: