Skip to content
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

Jump to bottom

bpo-42737: Mention PEP 563 in doc for annotated assignment #25511

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

bpo-42737: Mention PEP 563 in doc for annotated assignment #25511

wants to merge 3 commits into from
+14 −5

Conversation

gousaiyang
Copy link
Contributor

@gousaiyang gousaiyang commented Apr 21, 2021 •
edited by bedevere-bot

Currently, the documentation for variable annotations does not mention PEP 563 at all. This PR tries to improve this.

Note: this is describing the situation when #23952 is not applied, so that this PR can be easily backported to earlier versions of Python. We may need to either update #23952 itself or create a new PR to update the doc to demonstrate that annotations for complex targets will no longer be evaluated when PEP 563 is enabled.

https://bugs.python.org/issue42737

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Apr 21, 2021
@isidentical
Copy link
Sponsor Member

isidentical commented Apr 22, 2021

Please hold this PR until #23952 is resolved first.

@isidentical
Copy link
Sponsor Member

isidentical commented Apr 25, 2021

@gousaiyang could you please update the PR with the new changes (if necessary)

@gousaiyang
Copy link
Contributor Author

gousaiyang commented Apr 25, 2021

I think it might be better to merge this PR and create a separate one to document the changes in #23952, since the changes in this PR applies to (and thus can be backported) all Python versions down to 3.7, while changes in #23952 is for 3.10 only. If those changes are mixed in one PR, would that be harder to backport?

@github-actions
Copy link

github-actions bot commented Jun 3, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jun 3, 2021
JelleZijlstra
JelleZijlstra reviewed Apr 2, 2022
View changes
Doc/reference/simple_stmts.rst
created at the start of class or module body execution, if annotations
are found statically.
are found statically. If the ``annotations`` import from :mod:`__future__` is used
(i.e. :pep:`563` is enabled), the annotations are stored as string values in
Copy link
Member

@JelleZijlstra JelleZijlstra Apr 2, 2022

Choose a reason for hiding this comment

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

Ideally we should link to the documentation for the __future__ statement, not to the PEP directly. A PEP is a change proposal, not live documentation.

Doc/reference/simple_stmts.rst
attribute :attr:`__annotations__`
that is a dictionary mapping from variable names (mangled if private) to
evaluated annotations. This attribute is writable and is automatically
that is a dictionary mapping from variable names (mangled if private) to the
Copy link
Member

@JelleZijlstra JelleZijlstra Apr 2, 2022

Choose a reason for hiding this comment

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

Suggested change
that is a dictionary mapping from variable names (mangled if private) to the
that is a dictionary mapping from variable names (mangled if private) to

Reads better without the article

Doc/reference/simple_stmts.rst

For expressions as assignment targets, the annotations are evaluated if
in class or module scope, but not stored.
in class or module scope (regardless of whether :pep:`563` is enabled), but not
Copy link
Member

@JelleZijlstra JelleZijlstra Apr 2, 2022

Choose a reason for hiding this comment

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

Really? That seems like a bug.

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 6, 2022
hauntsaninja
hauntsaninja requested changes Jan 4, 2023
View changes
Copy link
Contributor

@hauntsaninja hauntsaninja left a comment

Could you address Jelle's review? The claim is false, e.g. you don't see any print out when running

from __future__ import annotations
import sys

def f():
    print("!!!")

x: f()
print(sys.modules[__name__].__annotations__)

@bedevere-bot
Copy link

bedevere-bot commented Jan 4, 2023

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra

@hauntsaninja hauntsaninja

Assignees
No one assigned
Labels
awaiting changes docs Documentation in the Doc dir needs backport to 3.10 needs backport to 3.11 skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@gousaiyang @isidentical @bedevere-bot @JelleZijlstra @hauntsaninja @serhiy-storchaka @the-knights-who-say-ni @ezio-melotti