FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-46414: Add typing.reveal_type by JelleZijlstra · Pull Request #30646 · python/cpython · GitHub

/ cpython Public

bpo-46414: Add typing.reveal_type - #30646

Merged
gvanrossum merged 6 commits into
python:mainfrom
JelleZijlstra:revealtype
Feb 2, 2022
ConversationCommits (6)ChecksFiles changed
Merged

bpo-46414: Add typing.reveal_type#30646
gvanrossum merged 6 commits into
python:mainfrom
JelleZijlstra:revealtype

Conversation

JelleZijlstra commented Jan 17, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

sobolevn reviewed Jan 17, 2022
View reviewed changes
Comment thread Doc/library/typing.rst Outdated
Comment thread Lib/test/test_typing.py Outdated
Comment thread Misc/NEWS.d/next/Library/2022-01-17-10-00-02.bpo-46414.Ld0b_y.rst Outdated

sobolevn commented Jan 17, 2022

Copy link
Copy Markdown
Member

Thank you! This solves my old pain with redefining reveal_type as:

from typing import TYPE_CHECKING
if not TYPE_CHECKING:
    reveal_type = print

gvanrossum commented Jan 17, 2022

Copy link
Copy Markdown
Member

Hold on. This feels awkward, since previously this was a builtin. I don’t want to have to import it.

JelleZijlstra commented Jan 17, 2022

Copy link
Copy Markdown
Member Author

Hold on. This feels awkward, since previously this was a builtin. I don’t want to have to import it.

Type checkers would still allow using it without the import. You'll have the option to import it if you want to run your code or document to readers what reveal_type() means.

gvanrossum commented Jan 17, 2022

Copy link
Copy Markdown
Member

It needs more discussion. Please wait until I have a keyboard.

JelleZijlstra commented Jan 17, 2022

Copy link
Copy Markdown
Member Author

No hurry! I also opened a thread on typing-sig about this, that's probably the best place to discuss how it should work.

JelleZijlstra commented Jan 22, 2022

Copy link
Copy Markdown
Member Author

@gvanrossum based on the discussion on typing-sig, are you satisfied with this change now?

I'm planning to also add reveal_locals() in a separate PR.

JelleZijlstra commented Jan 22, 2022

Copy link
Copy Markdown
Member Author

Wait, I didn't address Nikita's review yet.

JelleZijlstra mentioned this pull request Jan 22, 2022
Closed

gvanrossum commented Jan 23, 2022

Copy link
Copy Markdown
Member

Did we bikeshed yet whether it should write to stdout or stderr?

JelleZijlstra commented Jan 23, 2022

Copy link
Copy Markdown
Member Author

Did we bikeshed yet whether it should write to stdout or stderr?

No, though there have been some suggestions about the output format; I just sent a typing-sig email about that too. Personally I don't care much but I'd like to keep it simple.

gvanrossum commented Jan 23, 2022

Copy link
Copy Markdown
Member

Did we bikeshed yet whether it should write to stdout or stderr?

No, though there have been some suggestions about the output format; I just sent a typing-sig email about that too. Personally I don't care much but I'd like to keep it simple.

I'm just thinking that sending the output to stderr would be more in line with the debug nature of the API.

JelleZijlstra commented Jan 23, 2022

Copy link
Copy Markdown
Member Author

Pushed a change to make it write to stderr instead.

gvanrossum reviewed Jan 23, 2022
View reviewed changes
Comment thread Doc/library/typing.rst Outdated
Update Doc/library/typing.rst
b2bbe06 
Co-authored-by: Guido van Rossum <guido@python.org>
sobolevn reviewed Jan 23, 2022
View reviewed changes
Comment thread Lib/typing.py
sys.modules[re.__name__] = re


def reveal_type(obj: T, /) -> T:

sobolevn Jan 23, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Do we use type hints in typing.py? 🤔

JelleZijlstra Jan 23, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If this is merged, we will :). I don't see any reason not to, especially in cases like this where the type is simple and helps document the behavior.

AlexWaygood Jan 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There are type annotations in a few lines of typing.py:

cpython/Lib/typing.py

Line 2531 in 270a091

def read(self, n: int = -1) -> AnyStr:

It's just about the only module in the stdlib in which I've spotted any type annotations other than importlib :)

JelleZijlstra mentioned this pull request Jan 23, 2022
Closed

JelleZijlstra commented Feb 2, 2022

Copy link
Copy Markdown
Member Author

@gvanrossum do you think this is ready to merge now? The discussion on typing-sig has died down. (You added the "Do not merge" label a while ago.)

I think the other new function PRs are also ready, but I can ping you on those later.

gvanrossum approved these changes Feb 2, 2022
View reviewed changes

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

gvanrossum merged commit abcc3d7 into python:main Feb 2, 2022

bedevere-bot commented Feb 2, 2022

Copy link
Copy Markdown

@gvanrossum: Please replace # with GH- in the commit message next time. Thanks!

gvanrossum commented Feb 2, 2022

Copy link
Copy Markdown
Member

Should probably post the resolution to typing-sig.

JelleZijlstra deleted the revealtype branch February 2, 2022 02:50
This was referenced Feb 3, 2022
Merged
Merged
Merged
AlexWaygood mentioned this pull request Apr 1, 2022
Closed
gkirchou mentioned this pull request Dec 18, 2023
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

sobolevn sobolevn left review comments
AlexWaygood AlexWaygood left review comments
gvanrossum gvanrossum approved these changes
Fidget-Spinner Awaiting requested review from Fidget-Spinner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL