Skip to content

bpo-32892: Use ast.Constant instead of specific constant AST types.#9445

Merged
serhiy-storchaka merged 8 commits intopython:masterfrom
serhiy-storchaka:ast-constant
Sep 27, 2018
Merged

bpo-32892: Use ast.Constant instead of specific constant AST types.#9445
serhiy-storchaka merged 8 commits intopython:masterfrom
serhiy-storchaka:ast-constant

Conversation

@serhiy-storchaka
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka commented Sep 20, 2018

Copy link
Copy Markdown
Member

@vstinner vstinner left a 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.

def parse_converter(self, annotation):
if (hasattr(ast, 'Constant') and
isinstance(annotation, ast.Constant) and
type(annotation.value) is str):
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.

"if isinstance(annotation, ast.Str):" doesn't work?

Copy link
Copy Markdown
Member Author

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.

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.

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

Copy link
Copy Markdown
Member Author

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.

@serhiy-storchaka serhiy-storchaka merged commit 3f22811 into python:master Sep 27, 2018
@serhiy-storchaka serhiy-storchaka deleted the ast-constant branch September 27, 2018 14:42
tacaswell added a commit to tacaswell/pystemd that referenced this pull request Apr 2, 2019
tacaswell added a commit to tacaswell/pystemd that referenced this pull request Apr 2, 2019
tacaswell added a commit to tacaswell/pystemd that referenced this pull request Apr 2, 2019
facebook-github-bot pushed a commit to systemd/pystemd that referenced this pull request Apr 2, 2019
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
hroncok added a commit to hroncok/mako that referenced this pull request Apr 11, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants