-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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-32892: Use ast.Constant instead of specific constant AST types. #9445
bpo-32892: Use ast.Constant instead of specific constant AST types. #9445
Conversation
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. Just a few minor comments.
@@ -3946,6 +3948,11 @@ def bad_node(self, node): | |||
self.function.parameters[parameter_name] = p | |||
|
|||
def parse_converter(self, annotation): | |||
if (hasattr(ast, 'Constant') and | |||
isinstance(annotation, ast.Constant) and | |||
type(annotation.value) is str): |
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.
"if isinstance(annotation, ast.Str):" doesn't work?
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 don't want to depend on deprecated classes. The initial version of my patch just removed ast.Str
. It will be removed at some time in future.
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 see, it makes sense.
* :mod:`ast` classes ``Num``, ``Str``, ``Bytes``, ``NameConstant`` and | ||
``Ellipsis`` are considered deprecated and will be removed in future Python | ||
versions. :class:`~ast.Constant` should be used instead. | ||
(Contributed by Serhiy Storchaka in :issue:`32892`.) |
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.
Should we mention the deprecation in ast.rst as well? Maybe near:
https://docs.python.org/dev/library/ast.html#abstract-grammar
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.
Good point! I didn't know good place for this, you have suggested me one.
The `Str` object is no longer in in `_ast` is py38, switch to using the public `ast` module instead. python/cpython#9445 https://bugs.python.org/issue32892
The `Str` object is no longer in in `_ast` is py38, switch to using the public `ast` module instead. python/cpython#9445 https://bugs.python.org/issue32892
The `Str` object is no longer in in `_ast` is py38, switch to using the public `ast` module instead. python/cpython#9445 https://bugs.python.org/issue32892
Summary: The `Str` object is no longer in in `_ast` is py38, switch to using the public `ast` module instead. python/cpython#9445 https://bugs.python.org/issue32892 Pull Request resolved: #33 Differential Revision: D14721271 Pulled By: aleivag fbshipit-source-id: 1367f300f74f037983e5f6e51f7af8990f416759
Since Python 3.8, class ast.Constant is used for all constants. Old classes ast.Num, ast.Str, ast.Bytes, ast.NameConstant and ast.Ellipsis are still available, but they will be removed in future Python releases. Constants have values, but not always ns. See sqlalchemy#287 (comment) See python/cpython#9445 Fixes Pylons/pyramid_mako#47
https://bugs.python.org/issue32892