Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add kind field to ast.Constant, to distinguish u"..." from "..." for type checkers #80461

Closed
gvanrossum opened this issue Mar 13, 2019 · 8 comments
Closed

Add kind field to ast.Constant, to distinguish u"..." from "..." for type checkers #80461

gvanrossum opened this issue Mar 13, 2019 · 8 comments
Assignees
@gvanrossum
Labels
3.8 only security fixes type-bug An unexpected behavior, bug, or error

Comments

@gvanrossum
Copy link
Member

BPO 36280
Nosy @gvanrossum, @serhiy-storchaka, @miss-islington
PRs
  • bpo-36280: Add Constant.kind field #12295
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/gvanrossum'
closed_at = <Date 2019-03-13.20:01:54.579>
created_at = <Date 2019-03-13.04:00:08.733>
labels = ['type-bug', '3.8']
title = 'Add kind field to ast.Constant, to distinguish u"..." from "..." for type checkers'
updated_at = <Date 2019-03-13.20:01:54.578>
user = 'https://github.com/gvanrossum'

    bugs.python.org fields:

    activity = <Date 2019-03-13.20:01:54.578>
actor = 'gvanrossum'
assignee = 'gvanrossum'
closed = True
closed_date = <Date 2019-03-13.20:01:54.579>
closer = 'gvanrossum'
components = []
creation = <Date 2019-03-13.04:00:08.733>
creator = 'gvanrossum'
dependencies = []
files = []
hgrepos = []
issue_num = 36280
keywords = ['patch']
message_count = 5.0
messages = ['337834', '337853', '337854', '337872', '337873']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'serhiy.storchaka', 'miss-islington']
pr_nums = ['12295']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36280'
versions = ['Python 3.8']
    @gvanrossum gvanrossum added the 3.8 only security fixes label Mar 13, 2019
    @gvanrossum gvanrossum self-assigned this Mar 13, 2019
    @gvanrossum gvanrossum added the type-bug An unexpected behavior, bug, or error label Mar 13, 2019
    @serhiy-storchaka
    Copy link
    Member

    Maybe use a special subclass of Constant for indicating string literals with the "u" prefix?

    @gvanrossum
    Copy link
    Member Author

    Maybe use a special subclass of Constant for indicating string literals with the "u" prefix?

    That would indeed be more convenient, as it requires fewer code and test changes. Are there examples of such classes in Python.asdl?

    @gvanrossum
    Copy link
    Member Author

    On second thought, even though it's a subclass, it feels less consistent than an extra attribute. So I'd rather keep the current approach (kind -> 'u' or None).

    @miss-islington
    Copy link
    Contributor

    New changeset 10f8ce6 by Miss Islington (bot) (Guido van Rossum) in branch 'master':
    bpo-36280: Add Constant.kind field (GH-12295)
    10f8ce6

    @gvanrossum
    Copy link
    Member Author

    Thanks for the review Serhiy!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @vasily-v-ryabov
    Copy link

    Please correct me if I'm wrong. This Constant.kind field is not used in mypy any more. Previously it was used in fastparse.py (intro by this commit: python/mypy@69a5a02), but now I don't see it in mypy/fastparse.py or anywhere else. Yes, this field is present in ast module interface, and I didn't try to search if it is used by some PyPI package. Maybe you know where it is used. Thanks!

    @gvanrossum
    Copy link
    Member Author

    @vasily-v-ryabov What is your question, or what action are you requesting we take?

    @vasily-v-ryabov
    Copy link

    vasily-v-ryabov commented Nov 2, 2023 •
    edited

    @gvanrossum Initially I wanted to remove this field as it looked outdated. I've even prepared draft commit vasily-v-ryabov@68906e0 but then I've found this thread and related commits, and also I understood it may potentially break backward compatibility. If there is no such risk, I'd suggest to create a PR or prepare some deprecation. Though this particular commit is not critical to me if this stuff is really used.

    The reason to me is making AST more compact which should be generally good for performance. I'm interested in improvements for AST and compiler related stuff in CPython for my project (hope it will be open in the future).

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

    @gvanrossum gvanrossum

    Labels
    3.8 only security fixes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @gvanrossum @serhiy-storchaka @vasily-v-ryabov @miss-islington