-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add stubs for typing_extensions module #1471
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
Add stubs for typing_extensions module #1471
Conversation
Also, this pull request will probably need to wait due to mypy's feature freeze? Typeshed can be updated independently of mypy, but perhaps that'll complicate things. Either way, here it is, to be reviewed whenever. |
import sys | ||
import typing | ||
from typing import ( | ||
ClassVar, ContextManager, Counter, DefaultDict, Deque, |
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.
To re-export this name, you need to say ClassVar as ClassVar
(this also goes for the rest of this file).
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.
Are you sure? The stub seemed to work when I tried using it with mypy -- all the types seemed to be correctly re-exported and I was able to successfully typecheck my test files. Or is there something I'm missing?
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 mypy doesn't implement that part of PEP 484 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.
Huh, you're right! I did not know PEP 484 said that. Fixed.
@@ -0,0 +1,28 @@ | |||
import sys | |||
import typing | |||
from typing import ClassVar as ClassVar |
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.
You're missing Type
, which is also in __all__
.
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.
Whoops! Fixed.
# Return type that indicates a function does not return. | ||
# This type is equivalent to the None type, but the no-op Union is necessary to | ||
# distinguish the None type from the None value. | ||
NoReturn = typing.Union[None] |
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.
Why not from typing import NoReturn as NoReturn
?
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 stubs for typing currently don't define NoReturn
. I've added a TODO note mentioning that in my new commit. (Alternatively, I could do from mypy_extensions import NoReturn as NoReturn
, though that seems a little backwards).
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.
Hm, any reason it isn't in typing.pyi
yet? Maybe we can just fix that in this PR. If you'd prefer not to, I'll just merge 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.
It could be my fault. I probably just forgot to add the stub when I added NoReturn
to typing
.
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.
...now that I think about it, I have no idea -- it seems like an oversight?
I'd prefer keeping that change separate from this PR to keep the git history clean -- I made a separate pull request to add NoReturn and can rebase/fix this one once the other one is merged? (Or vice-versa)
from typing import ChainMap as ChainMap | ||
|
||
if sys.version_info >= (3, 5): | ||
from typing import AsyncIterable as AsyncIterable |
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.
Missing Awaitable.
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.
Also fixed.
718fb48
to
4f7982c
Compare
@JelleZijlstra -- I've responded to your comments up above! |
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.
Thanks! LGTM now.
# | ||
# TODO: Replace with direct import from typing once NoReturn is added to | ||
# typing.pyi. | ||
NoReturn = typing.Union[None] |
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 merged your other PR, so the comment and this line can now be replaced.
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.
Cool, I updated this pull request.
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.
@JelleZijlstra Ping -- not sure if you saw my comment up above?
See latest comment in python/typing#443 for justification.
Changes made: - Add missing `Type` re-export - Add missing `Awaitable` re-export - Add TODO note to `NoReturn`.
4f7982c
to
ad65237
Compare
Oops, sorry for missing that. I'll merge it now. |
This pull request adds stubs for the
typing_extensions
module. The module is currently being reviewed, but its contents are mostly finalized at this point so I figured I might as well make this pull request now.