GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-38605: Revert making 'from __future__ import annotations' the default by pablogsal · Pull Request #25490 · python/cpython

Conversation

@pablogsal

@pablogsal pablogsal commented Apr 20, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue38605

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Fidget-Spinner and domdfcoding reacted with heart emoji
All reactions
  • ❤️ 2 reactions
@pablogsal
pablogsal requested review from ericvsmith, gvanrossum, ilevkivskyi, isidentical, lysnikolaou, markshannon and rhettinger as code owners April 20, 2021 21:18
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Apr 20, 2021
@bedevere-bot bedevere-bot added the awaiting core review label Apr 20, 2021
@pablogsal
bpo-38605: Revert making 'from __future__ import annotations' the def…
1e024af
…ault

This reverts commit 044a104, adapting
the code to changes that happened after it.
@pablogsal
pablogsal force-pushed the revert-annotations branch from f267ef4 to 1e024af Compare April 20, 2021 21:23
@gousaiyang

gousaiyang commented Apr 20, 2021

Copy link
Copy Markdown
Contributor

Hi @pablogsal, you may also want to revert my previous PR #25236 for this.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Apr 20, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Could you give a little bit context to explain the revert? I guess that it's related to the many threads on python-dev about it?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 20, 2021

Copy link
Copy Markdown
Member Author

Is explained in the NEWs entry. I plan to explain it also on the final commit :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal
Revert "bpo-38605: Update "Future statements" docs since PEP 563 is a…
65a3272
…lways enabled (pythonGH-25236)"

This reverts commit 1be456a.
@vstinner

vstinner commented Apr 20, 2021

Copy link
Copy Markdown
Member

Oh ok, thanks. Copy of the NEWS entry:

Revert making 'from future import annotations' the default. This follows
the Steering Council decision to postpone PEP 563 changes to at least Python
3.11. See the original email for more information regarding the decision:
https://mail.python.org/archives/list/[email protected]/thread/CLVXXPQ2T2LQ5MP2Y53VVQFCXYWQJHKZ/.
Patch by Pablo Galindo.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@isidentical

isidentical commented Apr 20, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@pablogsal also please bump the magic number (cached pycs might cause some issues) (it wasn't done in the initial PR, but rather get picked up on #22630)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 20, 2021

Copy link
Copy Markdown
Member Author

@pablogsal also please bump the magic number (cached pycs might cause some issues) (it wasn't done in the initial PR, but rather get picked up on #22630)

We should be able to go back a version, I think. Cached pycs for alpha versions should not take lots of consideration

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 20, 2021

Copy link
Copy Markdown
Member Author

We should be able to go back a version, I think. Cached pycs for alpha versions should not take lots of consideration

Crap, there has been quite a lot of changes since then. Ok, bumping it is!

isidentical reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Apr 20, 2021

Copy link
Copy Markdown
Member Author

@pablogsal also please bump the magic number (cached pycs might cause some issues) (it wasn't done in the initial PR, but rather get picked up on #22630)

Done in ef38777a1f

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal
Dance dance the magic dance
f70048d
@pablogsal
pablogsal force-pushed the revert-annotations branch from ef38777 to f70048d Compare April 20, 2021 22:17
@vstinner vstinner closed this Apr 20, 2021
@vstinner vstinner reopened this Apr 20, 2021
@vstinner

vstinner commented Apr 20, 2021

Copy link
Copy Markdown
Member

Oops sorry, I wanted to remove my comment rather posting it, but I closed the PR instead... I reopened the PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gousaiyang

gousaiyang commented Apr 20, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

We can also update the doc of __future__ module and Lib/__future__.py to mention that annotations feature is postponed to be "mandatory in 3.11".

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal pablogsal closed this Apr 21, 2021
@pablogsal pablogsal reopened this Apr 21, 2021
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 21, 2021
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 21, 2021
methane
methane reviewed Apr 21, 2021
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2021-04-20-22-17-47.bpo-38605.9eeCNZ.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 21, 2021
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 21, 2021
@methane

This comment has been minimized.

Sign in to view
@pablogsal

pablogsal commented Apr 21, 2021

Copy link
Copy Markdown
Member Author

cpython/Python/ceval.c

Lines 4363 to 4366 in d35eef3

if (oparg & 0x04) {
assert(PyTuple_CheckExact(TOP()));
func->func_annotations = POP();
}

This line should be changed to assert(PyTuple_CheckExact(TOP()) || PyDict_CheckExact(TOP()));.
I don't know why this pull request passes the test...

Are you taking #23316 into consideration?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@methane

methane commented Apr 21, 2021

Copy link
Copy Markdown
Member

Are you taking #23316 into consideration?

Yes. And I know understand why this assrtion passes.

def add(a: int, b: int) -> int:
    return a + b
  3           0 LOAD_CONST               0 ('a')
              2 LOAD_NAME                0 (int)
              4 LOAD_CONST               1 ('b')
              6 LOAD_NAME                0 (int)
              8 LOAD_CONST               2 ('return')
             10 LOAD_NAME                0 (int)
             12 BUILD_TUPLE              6
             14 LOAD_CONST               3 (<code object add at 0x7f30fc37b050, file "a.py", line 3>)
             16 LOAD_CONST               4 ('add')
             18 MAKE_FUNCTION            4 (annotations)
             20 STORE_NAME               1 (add)

Annotation is tuple even when PEP 563 is disabled. That's nice.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@python python deleted a comment from bedevere-bot Apr 21, 2021
@python python deleted a comment from bedevere-bot Apr 21, 2021
@pablogsal
pablogsal force-pushed the revert-annotations branch from ce789a0 to 51a6f52 Compare April 21, 2021 01:23
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 21, 2021
@bedevere-bot

bedevere-bot commented Apr 21, 2021

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 51a6f52234cf416f33a8f8ce625502dedd2bf0fd 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 21, 2021
@pablogsal @methane
Update Misc/NEWS.d/next/Core and Builtins/2021-04-20-22-17-47.bpo-386…
b6a88ed
…05.9eeCNZ.rst

Co-authored-by: Inada Naoki <[email protected]>
@pablogsal
pablogsal force-pushed the revert-annotations branch from 51a6f52 to b6a88ed Compare April 21, 2021 01:42
@pablogsal

pablogsal commented Apr 21, 2021

Copy link
Copy Markdown
Member Author

I still would like if someone can formally approve the PR :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra reviewed Apr 21, 2021
View reviewed changes
Comment thread Lib/test/test_dataclasses.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Tools/peg_generator/pegen/grammar_parser.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@Zac-HD Zac-HD mentioned this pull request Apr 21, 2021
Resolving TypedDict fails under Python 3.10 HypothesisWorks/hypothesis#2897
Closed
methane
methane approved these changes Apr 21, 2021
View reviewed changes

@methane methane left a comment

Copy link
Copy Markdown
Member

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 have not reviewed tests yet. In other parts, LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Apr 21, 2021
vstinner
vstinner approved these changes Apr 21, 2021
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

LGTM but please address first the comment in Lib/test/test_dataclasses.py (I concur that it looks a revert of a bugfix, so it reintroduces a typo).

I suggest to first revert the change, and then write a second PR to announce that annotations are going to change again (in Python 3.11 or later).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@pablogsal

pablogsal commented Apr 21, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

I suggest to first revert the change, and then write a second PR to announce that annotations are going to change again (in Python 3.11 or later).

@vstinner What do you mean when you say "announce"? Announce where? In the What's new? A news entry? python-dev?

I suggest to first revert the change

Notice that this was certainly not a clean revert

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal
Fix dataclasses bug
82c1f24
@vstinner

vstinner commented Apr 21, 2021

Copy link
Copy Markdown
Member

@vstinner What do you mean when you say "announce"? Announce where? In the What's new? A news entry? python-dev?

Add a What's New in Python 3.10 entry to explain that the behavior changed but was then revert, and that it will change again in a future Python version.

Something like this announcement of future collections incompatible changes (collections.Mapping alias):
https://docs.python.org/dev/whatsnew/3.9.html#you-should-check-for-deprecationwarning-in-your-code

This announce was correct: the aliases were removed again in Python 3.10.

python-dev?

Any additional communication would be good, but the minimum would be a note in the What's New in Python 3.10.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner approved these changes Apr 21, 2021
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@pablogsal
pablogsal merged commit b0544ba into python:master Apr 21, 2021
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 21, 2021
@pablogsal
pablogsal deleted the revert-annotations branch April 21, 2021 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@vstinner vstinner vstinner approved these changes
@methane methane methane approved these changes
@ericvsmith ericvsmith Awaiting requested review from ericvsmith
@gvanrossum gvanrossum Awaiting requested review from gvanrossum
@ilevkivskyi ilevkivskyi Awaiting requested review from ilevkivskyi
@isidentical isidentical Awaiting requested review from isidentical
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou
@markshannon markshannon Awaiting requested review from markshannon
@rhettinger rhettinger Awaiting requested review from rhettinger

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@pablogsal @gousaiyang @vstinner @isidentical @methane @bedevere-bot @JelleZijlstra @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.