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 — gh-90953: Emit deprecation warnings for `ast` features deprecated in Python 3.8 by AlexWaygood · Pull Request #104199 · python/cpython

Conversation

@AlexWaygood

@AlexWaygood AlexWaygood commented May 5, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This is a continuation of @serhiy-storchaka's PR #31432, but removes the deprecation warnings for ast.ExtSlice and ast.Index, which were only deprecated in Python 3.9 (rationale for excluding them: #31432 (comment)).

  • Issue: ast.Constant.n deprecated without warning #90953

📚 Documentation preview 📚: https://cpython-previews--104199.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
serhiy-storchaka and others added 11 commits February 19, 2022 14:20
@serhiy-storchaka
bpo-46797: Emit deprecation warnings for deprecated ast features
8de6eba
@serhiy-storchaka
Merge branch 'main' into ast-deprecation-warnings
7908047
@iritkatriel
Merge branch 'main' into ast-deprecation-warnings
dfedc35
@serhiy-storchaka
Merge branch 'main' into ast-deprecation-warnings
b2ca297
@iritkatriel
Merge branch 'main' into ast-deprecation-warnings
fbd02f3
@hugovk
Merge branch 'main' into ast-deprecation-warnings
4320111
@AlexWaygood @hugovk
fix patchcheck
fdefe87 
Co-authored-by: Hugo van Kemenade <[email protected]>
@AlexWaygood
Remove DeprecationWarnings for Index and ExtSlice
60ab0c4
@AlexWaygood
Also emit DeprecationWarning during isinstance() checks
951c548
@AlexWaygood
Add whatsnew
9025b7e
@AlexWaygood
Tweak news entry
3d2de74
@AlexWaygood AlexWaygood added type-feature A feature request or enhancement stdlib Standard Library Python modules in the Lib/ directory 3.12 only security fixes labels May 5, 2023
@AlexWaygood
AlexWaygood requested review from hugovk and serhiy-storchaka May 5, 2023 12:03
@AlexWaygood
AlexWaygood requested a review from isidentical as a code owner May 5, 2023 12:03
@bedevere-bot bedevere-bot mentioned this pull request May 5, 2023
ast.Constant.n deprecated without warning #90953
Closed
@bedevere-bot bedevere-bot added the awaiting core review label May 5, 2023
@AlexWaygood
Merge branch 'main' into ast-deprecation-warnings
4b818b6
AlexWaygood
AlexWaygood commented May 5, 2023
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
hugovk
hugovk approved these changes May 5, 2023
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Library/2022-02-19-14-19-34.bpo-46797.6BXZX4.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels May 5, 2023
AlexWaygood and others added 4 commits May 5, 2023 13:48
@AlexWaygood @hugovk
Apply suggestions from code review
6e9f5f5 
Co-authored-by: Hugo van Kemenade <[email protected]>
@AlexWaygood
Use `warnings._deprecated
8f86b9a
@AlexWaygood
Merge branch 'ast-deprecation-warnings' of https://github.com/AlexWay…
7e4cf89 
…good/cpython into ast-deprecation-warnings
@AlexWaygood
Move whatsnew entry to correct location
8c949f3
@AlexWaygood AlexWaygood mentioned this pull request May 5, 2023
gh-90953: Emit deprecation warnings for deprecated ast features #31432
Closed
@AlexWaygood
Merge branch 'main' into ast-deprecation-warnings
4a7a9c1
AlexWaygood and others added 6 commits May 5, 2023 14:19
@AlexWaygood
Revert spurious whitespace change
3d71e66
@AlexWaygood
Revert warning for dims attribute (only deprecated in 3.9)
4261cc6
@AlexWaygood
Also remove deprecation for ast.slice, deprecated in 3.9
a785a0c
@AlexWaygood
Better unittest style
a3491ea
@AlexWaygood
.
3613908
@AlexWaygood
Update 3.12.rst
9bba5dd
hugovk
hugovk reviewed May 5, 2023
View reviewed changes
Comment thread Lib/test/test_ast.py
self.assertIsInstance(n, N)
self.assertIsinstance(n, ast.Num)
self.assertNotIsInstance(n, N2)
self.assertNotIsInstance(ast.Num(42), N)

@hugovk hugovk May 5, 2023

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.

Nice :)

By the way, https://github.com/isidentical/teyit / https://pypi.org/project/teyit/ is a nice tool that can upgrade some of these automatically.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@AlexWaygood
typo
234243a
barneygale
barneygale reviewed May 5, 2023
View reviewed changes

@barneygale barneygale left a comment

Copy link
Copy Markdown
Contributor

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.

Is it necessary to emit deprecation warnings when the names are accessed on the ast module? Would it not be sufficient to emit warnings when these classes are instantiated (+ isinstance etc)?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/ast.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/ast.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
Add comment explaining subtle code
915e12e
@AlexWaygood

AlexWaygood commented May 6, 2023

Copy link
Copy Markdown
Member Author

Is it necessary to emit deprecation warnings when the names are accessed on the ast module? Would it not be sufficient to emit warnings when these classes are instantiated (+ isinstance etc)?

I guess I lean towards keeping them just to ensure maximum visibility. There are edge cases where you might not be "using" the class or calling isinstance() on it, but you do reference it (if type(blah) is ast.Num, etc.). But you are right that we could probably simplify the code somewhat if we only warned on instantiation and isinstance() checks.

@hugovk, do you have any thoughts on this?
barneygale 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.
@hugovk

hugovk commented May 6, 2023

Copy link
Copy Markdown
Member

If there are two common ways of accessing something, I'd also lean towards more visibility for both.

Speaking from a position of having dealt with the fallout of removing something without a DeprecationWarning first, just mentioning in docs :)
AlexWaygood and barneygale reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hugovk
hugovk approved these changes May 6, 2023
View reviewed changes
AlexWaygood added 2 commits May 6, 2023 16:58
@AlexWaygood
Simplify slightly
a7abed2
@AlexWaygood
Unnecessary f-string
7ceb9cc
barneygale
barneygale approved these changes May 6, 2023
View reviewed changes
@AlexWaygood
One last nit
ecf092f
@AlexWaygood
AlexWaygood enabled auto-merge (squash) May 6, 2023 16:20
@AlexWaygood

AlexWaygood commented May 6, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

Thanks very much, both of you!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
Merge branch 'main' into ast-deprecation-warnings
dd3e1e1
@AlexWaygood
AlexWaygood merged commit 376137f into python:main May 6, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label May 6, 2023
@AlexWaygood
AlexWaygood deleted the ast-deprecation-warnings branch May 6, 2023 16:51
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
@AlexWaygood @serhiy-storchaka @jbower-fb
pythongh-90953: Emit deprecation warnings for ast features deprecat…
1173ce2 
…ed in Python 3.8 (python#104199)

`ast.Num`, `ast.Str`, `ast.Bytes`, `ast.Ellipsis` and `ast.NameConstant` now all emit deprecation warnings on import, access, instantation or `isinstance()` checks.

Co-authored-by: Serhiy Storchaka <[email protected]>
@seifertm seifertm mentioned this pull request May 9, 2023
[Python 3.12] Assertion rewriting triggers DeprecationWarning pytest-dev/pytest#10977
Closed
@iritkatriel

iritkatriel commented May 24, 2024

Copy link
Copy Markdown
Member

@AlexWaygood Is it time to make the PR to remove them now?
All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood commented May 25, 2024

Copy link
Copy Markdown
Member Author

@AlexWaygood Is it time to make the PR to remove them now?

yes
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood AlexWaygood mentioned this pull request May 25, 2024
Remove AST nodes deprecated since Python 3.8, with warnings since Python 3.12 #119562
Closed
@AlexWaygood

AlexWaygood commented May 25, 2024

Copy link
Copy Markdown
Member Author

@AlexWaygood Is it time to make the PR to remove them now?

See #119563
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@barneygale barneygale barneygale approved these changes
@hugovk hugovk hugovk approved these changes
@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka
@isidentical isidentical Awaiting requested review from isidentical isidentical is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

3.12 only security fixes stdlib Standard Library Python modules in the Lib/ directory type-feature A feature request or enhancement

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.

6 participants

@AlexWaygood @hugovk @iritkatriel @barneygale @bedevere-bot @serhiy-storchaka
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.