-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
bpo-38605: Revert making 'from __future__ import annotations' the default #25490
Conversation
…ault This reverts commit 044a104, adapting the code to changes that happened after it.
f267ef4
to
1e024af
Compare
Hi @pablogsal, you may also want to revert my previous PR #25236 for this. |
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? |
Is explained in the NEWs entry. I plan to explain it also on the final commit :) |
…lways enabled (pythonGH-25236)" This reverts commit 1be456a.
Oh ok, thanks. Copy of the NEWS entry: Revert making 'from future import annotations' the default. This follows |
@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 |
Crap, there has been quite a lot of changes since then. Ok, bumping it is! |
Done in ef38777a1f |
ef38777
to
f70048d
Compare
Oops sorry, I wanted to remove my comment rather posting it, but I closed the PR instead... I reopened the PR. |
We can also update the doc of |
Misc/NEWS.d/next/Core and Builtins/2021-04-20-22-17-47.bpo-38605.9eeCNZ.rst
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Are you taking #23316 into consideration? |
Yes. And I know understand why this assrtion passes.
Annotation is tuple even when PEP 563 is disabled. That's nice. |
ce789a0
to
51a6f52
Compare
🤖 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. |
…05.9eeCNZ.rst Co-authored-by: Inada Naoki <songofacandy@gmail.com>
51a6f52
to
b6a88ed
Compare
I still would like if someone can formally approve the PR :) |
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.
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).
@vstinner What do you mean when you say "announce"? Announce where? In the What's new? A news entry? python-dev?
Notice that this was certainly not a clean revert |
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): This announce was correct: the aliases were removed again in Python 3.10.
Any additional communication would be good, but the minimum would be a note in the What's New in Python 3.10. |
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.
https://bugs.python.org/issue38605