-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[docs] Update common_issues.rst: update information about reveal type & locals #19059
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
base: master
Are you sure you want to change the base?
Conversation
…veal locals Previously it was impossible to have these in at run time (more or less), but now you can just import them.
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 tried to find the history of this decision but gave up.
Unsurprising since this was a CPython decision without a PEP or anything :^)
I'm going to guess it was either forgotten or discarded because other type checkers don't implement it. See python/cpython#90572
(this review is just a bunch of nitpicks)
and don't have to be imported as functions. However, | ||
if you don't import them, then they don't exist at runtime! Therefore, | ||
you'll have to remove any ``reveal_type`` and ``reveal_locals`` calls from your program | ||
or else Python will give you an error at runtime about those names being undefined. |
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 ragged right side makes me unhappy :-(
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 text it replaces was exceptionally non-ragged, as well! What a shame.
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.
Anyway, I deliberately broke it up by clause, (like eg https://sembr.org/ ) which leads to exceptional raggedness. But theoretically will make diffs easier to look at when someone edits this in the future.
(Although, technically, if you really didn't want to remove those calls, you could use | ||
``if not typing.TYPE_CHECKING: reveal_locals = lambda: None`` | ||
or similar to define the function to something else 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.
IMO this is unnecessary detail and people could come up with this if they need 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.
I thought about that when I was writing it, but I like it when documentation errs on the side of telling me too much, instead of too little (which is the dominant failure mode of documentation). And, besides: anyone reading "Common issues and solutions"§"Displaying the type of an expression" probably wants all the help they can get.
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.
Makes sense, unfortunately I can't mark this as resolved.
you'll have to remove any ``reveal_type`` and ``reveal_locals`` calls from your program | ||
or else Python will give you an error at runtime about those names being undefined. | ||
Alternatively, you can import ``reveal_type`` from ``typing_extensions`` | ||
(or, in more recent versions of python, from ``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.
Does intersphinx work here? Could we link to typing.reveal_type
?
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 don't know why, but this doesn't seem to work for me. Even though the intersphinices to sys.version_info and typing.TYPE_CHECKING seem to work (even in the same spot), typing.reveal_type won't generate for me. Maybe I'm doing something wrong locally (there's a good chance I don't know what I'm doing) or there's some caching problem I'm running into. Anyway, it's harmless to include an intersphinx to typing.reveal_type here because it gracefully degrades down to normal code
, so I will.
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.
Ah, nevermind: I actually get a hard error when I try to push that: py:data reference target not found: typing.reveal_type
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.
Unfortunately I can't mark this as resolved :(
I believe this was on the typing-sig mailing list. I initially proposed adding |
it doesn't in my local testing, but that's ok because it gracefully degrades down to `normal code`
Ah, nevermind: I actually get a hard error when I try to push that: `py:data reference target not found: typing.reveal_type`. This reverts commit a852106.
Previously it was impossible to have these in at run time (more or less), but now you can just import one of them.
Tangential open question I'm curious about: Is there any interest in getting reveal_locals into typing? I've never used it myself, so I have no strong opinions one way or the other, but I'm surprised reveal_type made it in and reveal_locals didn't! I tried to find the history of this decision but gave up.