bpo-29974: Change typing.TYPE_CHECKING doc example#982
bpo-29974: Change typing.TYPE_CHECKING doc example#982berkerpeksag merged 1 commit intopython:masterfrom
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! |
berkerpeksag
left a comment
There was a problem hiding this comment.
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? |
There was a problem hiding this comment.
Nitpick: Parentheses are not needed.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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".