Skip to content

gh-90953: Emit deprecation warnings for ast features deprecated in Python 3.8 - #104199

Merged
AlexWaygood merged 29 commits into
python:mainfrom
AlexWaygood:ast-deprecation-warnings
May 6, 2023
Merged

gh-90953: Emit deprecation warnings for ast features deprecated in Python 3.8#104199
AlexWaygood merged 29 commits into
python:mainfrom
AlexWaygood:ast-deprecation-warnings

Conversation

@AlexWaygood

@AlexWaygoodAlexWaygood commented May 5, 2023•
edited
Loading

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)).

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

@AlexWaygoodAlexWaygood 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
@bedevere-botbedevere-bot mentioned this pull request May 5, 2023
Closed
AlexWaygood
AlexWaygood commented May 5, 2023
View reviewed changes
Comment threadDoc/whatsnew/3.12.rst Outdated
hugovk
hugovk approved these changes May 5, 2023
View reviewed changes
Comment threadDoc/whatsnew/3.12.rst Outdated
Comment threadDoc/whatsnew/3.12.rst Outdated
Comment threadMisc/NEWS.d/next/Library/2022-02-19-14-19-34.bpo-46797.6BXZX4.rst Outdated
@AlexWaygoodAlexWaygood mentioned this pull request May 5, 2023
Closed
hugovk
hugovk reviewed May 5, 2023
View reviewed changes
Comment threadLib/test/test_ast.py
self.assertIsInstance(n, N)
self.assertIsinstance(n, ast.Num)
self.assertNotIsInstance(n, N2)
self.assertNotIsInstance(ast.Num(42), N)

@hugovkhugovkMay 5, 2023

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.

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.

barneygale
barneygale reviewed May 5, 2023
View reviewed changes

@barneygalebarneygale left a comment

Copy link
Copy Markdown
Contributor

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)?

Comment threadLib/ast.py
Comment threadLib/ast.py Outdated
@AlexWaygood

AlexWaygood commented May 6, 2023

Copy link
Copy Markdown
MemberAuthor

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?

@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
AlexWaygood enabled auto-merge (squash) May 6, 2023 16:20
@AlexWaygood

AlexWaygood commented May 6, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Thanks very much, both of you!

@AlexWaygood
AlexWaygood merged commit 376137f into python:mainMay 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 forastfeatures 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 <storchaka@gmail.com>
@seifertmseifertm mentioned this pull request May 9, 2023
Closed
@iritkatriel

iritkatriel commented May 24, 2024

Copy link
Copy Markdown
Member

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

@AlexWaygood

AlexWaygood commented May 25, 2024

Copy link
Copy Markdown
MemberAuthor

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

yes

@AlexWaygoodAlexWaygood mentioned this pull request May 25, 2024
Closed
@AlexWaygood

AlexWaygood commented May 25, 2024

Copy link
Copy Markdown
MemberAuthor

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

See #119563

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

Reviewers

@barneygalebarneygalebarneygale approved these changes
@hugovkhugovkhugovk approved these changes
@serhiy-storchakaserhiy-storchakaAwaiting requested review from serhiy-storchaka
@isidenticalisidenticalAwaiting requested review from isidenticalisidentical is a code owner

Assignees

No one assigned

Labels

3.12only security fixesstdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@AlexWaygood@hugovk@iritkatriel@barneygale@bedevere-bot@serhiy-storchaka