Skip to content

Improve the AST documentation #83380

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

Closed
pablogsal opened this issue Jan 3, 2020 · 7 comments
Closed

Improve the AST documentation #83380

pablogsal opened this issue Jan 3, 2020 · 7 comments
Labels
3.9 only security fixes docs Documentation in the Doc dir

Comments

@pablogsal
Copy link
Member

BPO 39199
Nosy @serhiy-storchaka, @pablogsal
PRs
  • bpo-39199: Add descriptions of non-deprecated AST nodes to the AST module documentation #17812
  • bpo-39199: Use 'eval' mode for the examples with expression nodes #18828
  • bpo-39199: Add attribution to the Green Tree Snakes in the AST docs #24727
  • 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 = None
    closed_at = None
    created_at = <Date 2020-01-03.13:05:21.001>
    labels = ['3.9', 'docs']
    title = 'Improve the AST documentation'
    updated_at = <Date 2021-03-03.16:43:03.193>
    user = 'https://github.com/pablogsal'

    bugs.python.org fields:

    activity = <Date 2021-03-03.16:43:03.193>
    actor = 'pablogsal'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2020-01-03.13:05:21.001>
    creator = 'pablogsal'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39199
    keywords = ['patch']
    message_count = 7.0
    messages = ['359235', '363112', '363585', '363604', '363611', '363777', '363783']
    nosy_count = 3.0
    nosy_names = ['docs@python', 'serhiy.storchaka', 'pablogsal']
    pr_nums = ['17812', '18828', '24727']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue39199'
    versions = ['Python 3.9']

    @pablogsal
    Copy link
    Member Author

    The AST docs need some love as they can be a bit obscure to someone new to the module. Improvements to be considered in this issue:

    • Document all available nodes (as of 3.8 and not deprecated ones). This helps to know what classes to consider when implementing methods for the visitors.

    • Add some short practical examples for the visitors: one to query an AST and another to modify it.

    @pablogsal pablogsal added the 3.9 only security fixes label Jan 3, 2020
    @pablogsal pablogsal added docs Documentation in the Doc dir 3.9 only security fixes labels Jan 3, 2020
    @pablogsal pablogsal added the docs Documentation in the Doc dir label Jan 3, 2020
    @pablogsal
    Copy link
    Member Author

    New changeset 114081f by Pablo Galindo in branch 'master':
    bpo-39199: Add descriptions of non-deprecated nodes to the AST module documentation (GH-17812)
    114081f

    @serhiy-storchaka
    Copy link
    Member

    Would not be better to use mode='eval' for expression nodes?

            >>> print(ast.dump(ast.parse('123', mode='eval'), indent=4))
            Expression(
                body=Constant(value=123, kind=None))

    @pablogsal
    Copy link
    Member Author

    Would not be better to use mode='eval' for expression nodes?

    Agreed! I will prepare a PR soon to simplify the expression examples.

    @pablogsal
    Copy link
    Member Author

    New changeset 02f64cb by Pablo Galindo in branch 'master':
    bpo-39199: Use 'eval' mode for the examples with expression nodes (GH-18828)
    02f64cb

    @serhiy-storchaka
    Copy link
    Member

    As you worked much with ast.dump(), what multi-line formatting do you prefer, the current

        Module(
            body=[
                If(
                    test=Name(id='x', ctx=Load()),
                    body=[
                        Expr(
                            value=Constant(value=Ellipsis))],
                    orelse=[
                        If(
                            test=Name(id='y', ctx=Load()),
                            body=[
                                Expr(
                                    value=Constant(value=Ellipsis))],
                            orelse=[
                                Expr(
                                    value=Constant(value=Ellipsis))])])],
            type_ignores=[])
    

    or with closing brackets on separate lines?

        Module(
            body=[
                If(
                    test=Name(id='x', ctx=Load()),
                    body=[
                        Expr(
                            value=Constant(value=Ellipsis)
                        )
                    ],
                    orelse=[
                        If(
                            test=Name(id='y', ctx=Load()),
                            body=[
                                Expr(
                                    value=Constant(value=Ellipsis)
                                )
                            ],
                            orelse=[
                                Expr(
                                    value=Constant(value=Ellipsis)
                                )
                            ]
                        )
                    ]
                )
            ],
            type_ignores=[]
        )
    

    The latter make contain long stairs of closing brackets.

    @pablogsal
    Copy link
    Member Author

    The first one looks on first inspection "cleaner" but then I tried to look at a random closed bracket/parenthesis like the ones in

                                        
        value=Constant(value=Ellipsis))])])],

    and trying to guess where that closes and is confusing to say the least so I think I would prefer the second one as is less "dense".

    Additionally, I was curious and I have asked several people with different examples and almost everyone prefers the second one, being
    the only reason someone preferred the first the fact that "fits vertically in the screen and you need less scrolling to read it all".

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants