python / cpython

The Python programming language
https://www.python.org
Other
63.11k stars 30.22k forks source link

Output closing parenthesis in ast.dump() on separate line #84170

Open serhiy-storchaka opened 4 years ago

serhiy-storchaka commented 4 years ago
BPO 39989
Nosy @rhettinger, @terryjreedy, @mdickinson, @ericvsmith, @benjaminp, @serhiy-storchaka, @asottile, @pablogsal
PRs
  • python/cpython#19039
  • 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: ```python assignee = None closed_at = None created_at = labels = ['type-feature', 'library', '3.9'] title = 'Output closing parenthesis in ast.dump() on separate line' updated_at = user = 'https://github.com/serhiy-storchaka' ``` bugs.python.org fields: ```python activity = actor = 'Anthony Sottile' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'serhiy.storchaka' dependencies = [] files = [] hgrepos = [] issue_num = 39989 keywords = ['patch'] message_count = 6.0 messages = ['364391', '364411', '364426', '364427', '364457', '364824'] nosy_count = 8.0 nosy_names = ['rhettinger', 'terry.reedy', 'mark.dickinson', 'eric.smith', 'benjamin.peterson', 'serhiy.storchaka', 'Anthony Sottile', 'pablogsal'] pr_nums = ['19039'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue39989' versions = ['Python 3.9'] ```

    serhiy-storchaka commented 4 years ago

    Currently ast.dump() in multiline mode (see bpo-37995) appends closing parenthesis to the end of the line:

    >>> import ast
    >>> node = ast.parse('spam(eggs, "and cheese")')
    >>> print(ast.dump(node, indent=3))
    Module(
       body=[
          Expr(
             value=Call(
                func=Name(id='spam', ctx=Load()),
                args=[
                   Name(id='eggs', ctx=Load()),
                   Constant(value='and cheese')],
                keywords=[]))],
       type_ignores=[])

    It uses vertical space more efficiently (which is especially important on Windows console).

    But I got a feedback about output closing parenthesis on separate lines (msg363783):

    Module(
       body=[
          Expr(
             value=Call(
                func=Name(id='spam', ctx=Load()),
                args=[
                   Name(id='eggs', ctx=Load()),
                   Constant(value='and cheese')
                ],
                keywords=[]
             )
          )
       ],
       type_ignores=[]
    )

    It looks more "balanced", but less vertical space efficient. It adds almost 300 lines to 57 examples in Doc/library/ast.rst. And after omitting optional list arguments like keywords=[] and type_ignores=[] (see bpo-39981) the stairs of parenthesis will look even longer.

    The proposed PR changes the output of ast.dump() by moving closing parenthesis on separate lines. I am still not sure what output is better.

    mdickinson commented 4 years ago

    This feels like something that's very much down to personal preference.

    I also prefer the closing "]" and ")" characters on their own lines, but I'd be happy with an argument to ast.dump giving me that option.

    ericvsmith commented 4 years ago

    For what it's worth (which might not be much), here is what black produces:

    Module(
        body=[
            Expr(
                value=Call(
                    func=Name(id="spam", ctx=Load()),
                    args=[Name(id="eggs", ctx=Load()), Constant(value="and cheese")],
                    keywords=[],
                )
            )
        ],
        type_ignores=[],
    )

    I agree with Mark: it's all probably personal preference, and I'd be okay either way.

    mdickinson commented 4 years ago

    Ah yes; looking at the black output, there's also an opportunity for an exciting discussion about trailing commas here :-)

    terryjreedy commented 4 years ago

    The current output looks like Python code. The proposed revision looks more like C, and I find the example above less readable with the prominence given to what is close to noise. The difference is part of the reason I left C for Python over 2 decades ago. Please make the alternative an option. The preferred form depends on the person and possibly the AST. (The example in Pablo's message is quite different.)

    341ce0e2-bdbb-4bd4-99e6-746f11201a3f commented 4 years ago

    fwiw, astpretty's output looks like the black output: https://github.com/asottile/astpretty

    >>> astpretty.pprint(ast.parse('if x == y: y += 4').body[0])
    If(
        lineno=1,
        col_offset=0,
        test=Compare(
            lineno=1,
            col_offset=3,
            left=Name(lineno=1, col_offset=3, id='x', ctx=Load()),
            ops=[Eq()],
            comparators=[Name(lineno=1, col_offset=8, id='y', ctx=Load())],
        ),
        body=[
            AugAssign(
                lineno=1,
                col_offset=11,
                target=Name(lineno=1, col_offset=11, id='y', ctx=Store()),
                op=Add(),
                value=Num(lineno=1, col_offset=16, n=4),
            ),
        ],
        orelse=[],
    )