gh-90953: Emit deprecation warnings for ast features deprecated in Python 3.8 by AlexWaygood · Pull Request #104199 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation20 Commits29 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

AlexWaygood

@serhiy-storchaka

@serhiy-storchaka

@iritkatriel

@serhiy-storchaka

@iritkatriel

@hugovk

@AlexWaygood @hugovk

Co-authored-by: Hugo van Kemenade hugovk@users.noreply.github.com

@AlexWaygood

@AlexWaygood

@AlexWaygood

@AlexWaygood

@AlexWaygood

AlexWaygood

hugovk

@AlexWaygood @hugovk

Co-authored-by: Hugo van Kemenade hugovk@users.noreply.github.com

@AlexWaygood

@AlexWaygood

@AlexWaygood

@AlexWaygood

hugovk

self.assertIsInstance(n, N)
self.assertIsinstance(n, ast.Num)
self.assertNotIsInstance(n, N2)
self.assertNotIsInstance(ast.Num(42), N)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexWaygood

barneygale

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

@AlexWaygood

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

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

hugovk

@AlexWaygood

@AlexWaygood

barneygale

@AlexWaygood

@AlexWaygood

Thanks very much, both of you!

@AlexWaygood

jbower-fb pushed a commit to jbower-fb/cpython-jbowerfb 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 storchaka@gmail.com

@iritkatriel

@AlexWaygood

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

yes

@AlexWaygood