Skip to content

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygoodAlexWaygood commented May 5, 2023

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
self.assertIsInstance(n, N)
self.assertIsinstance(n, ast.Num)
self.assertNotIsInstance(n, N2)
self.assertNotIsInstance(ast.Num(42), N)
Copy link
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.

Copy link
Contributor

@barneygalebarneygale left a 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)?

@AlexWaygood
Copy link
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
Copy link
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 :)

@AlexWaygoodAlexWaygood enabled auto-merge (squash) May 6, 2023 16:20
@AlexWaygood
Copy link
MemberAuthor

AlexWaygood commented May 6, 2023

Thanks very much, both of you!

@AlexWaygoodAlexWaygood merged commit 376137f into python:mainMay 6, 2023
@AlexWaygoodAlexWaygood 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
…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]>
@iritkatriel
Copy link
Member

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

@AlexWaygood
Copy link
MemberAuthor

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

yes

@AlexWaygood
Copy link
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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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