-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
bpo-29974: Change typing.TYPE_CHECKING doc example #982
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
@Mortal, thanks for your PR! By analyzing the history of the files in this pull request, we identified @berkerpeksag, @zware and @brettcannon to be potential reviewers. |
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at 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.
Looks good to me, but I'm not really a typing guru so I'm just pinging the author of the original commit. @ilevkivskyi could you confirm that the proposed patch is correct?
The original example is correct, annotations for local variables (i.e. in function scope) are not evaluated at runtime. The motivation for this was to reduce the possible runtime overhead of using annotations, but it also helps with forward references/import cycles. I think this example should stay. Maybe we could extend it with a function parameter annotation, to show that types should be string literals in the places where they are actually evaluated. For example something like this: if TYPE_CHECKING:
import expensive_mod
def fun(arg: 'expensive_mod.some_type') -> None:
local_var: expensive_mod.another_type = other_fun() |
Also it is probably worth fixing PEP 8 here and use |
@ilevkivskyi Thanks for the explanation -- I did not know that type annotations on local variables in function scope are not evaluated. I had naively tested with a type annotation on a variable in module-scope. (I guess it makes sense to ensure that a type annotation is evaluated at most once.) I will update the patch with @ilevkivskyi's proposed changes. |
@berkerpeksag It seems GitHub thinks your approval of the old patch still applies even though I've rewritten the entire patch. Is my addition of a short recap of PEP 484/526 OK? |
Doc/library/typing.rst
Outdated
def fun(arg: 'expensive_mod.SomeType') -> None: | ||
local_var: expensive_mod.AnotherType = other_fun() | ||
|
||
(Note that the first type annotation must be enclosed in quotes, making it a |
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.
Nitpick: Parentheses are not needed.
Doc/library/typing.rst
Outdated
|
||
(Note that the first type annotation must be enclosed in quotes, making it a | ||
"forward reference", to hide the expensive_mod reference from the | ||
interpreter runtime. Type annotations for local variables are not evaluated, |
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.
Please add two spaces after a full stop. See http://cpython-devguide.readthedocs.io/documenting.html#use-of-whitespace for details.
Please keep the issue number in the PR description. I added |
I've updated the docs according to review from @berkerpeksag. Sorry about removing the issue number in the PR title. |
Doc/library/typing.rst
Outdated
local_var: expensive_mod.AnotherType = other_fun() | ||
|
||
Note that the first type annotation must be enclosed in quotes, making it a | ||
"forward reference", to hide the expensive_mod reference from the |
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 would propose to use back-quotes for expensive_mod here. Like this:
``expensive_mod``
Otherwise LGTM.
* Fix PEP 8 (SomeType instead of some_type) * Add a function parameter annotation * Explain, using wording from PEP 484 and PEP 526, why one annotation is in quotes and another is not. Suggested by Ivan Levkevskyi.
Good catch about using backquotes. I've updated according to review from @ilevkivskyi and squashed to a single commit. |
Thanks! |
* Fix PEP 8 (SomeType instead of some_type) * Add a function parameter annotation * Explain, using wording from PEP 484 and PEP 526, why one annotation is in quotes and another is not. Suggested by Ivan Levkevskyi. (cherry picked from commit 87c07fe)
* Fix PEP 8 (SomeType instead of some_type) * Add a function parameter annotation * Explain, using wording from PEP 484 and PEP 526, why one annotation is in quotes and another is not. Suggested by Ivan Levkevskyi. (cherry picked from commit 87c07fe)
* Fix PEP 8 (SomeType instead of some_type) * Add a function parameter annotation * Explain, using wording from PEP 484 and PEP 526, why one annotation is in quotes and another is not. Suggested by Ivan Levkevskyi. (cherry picked from commit 87c07fe)
* Fix PEP 8 (SomeType instead of some_type) * Add a function parameter annotation * Explain, using wording from PEP 484 and PEP 526, why one annotation is in quotes and another is not. Suggested by Ivan Levkevskyi. (cherry picked from commit 87c07fe)
The documentation of typing.TYPE_CHECKING has an example (introduced in issue #26141) that would lead to NameError at runtime. The example shows how to limit the import of "expensive_mod" to type checkers, but then goes on to use "expensive_mod.some_type" in a type annotation that is evaluated at runtime ("local_var: expensive_mod.some_type"). The use case of TYPE_CHECKING is probably meant for type annotations placed in comments, e.g. "local_var # type: expensive_mod.some_type".