search

google / yapf

A formatter for Python files
Apache License 2.0
13.76k stars 890 forks source link

Yapf gets confused with a long list of tuples #478

Open jleclanche opened 6 years ago

jleclanche commented 6 years ago

Test case:

Write the following file to foo.py (note the tab indents):

(
    [
        (0, "INVALID"), (1, "TEST_TEMPORARY"), (2, "CORE"), (3, "EXPERT1"), (4, "HOF"),
        (5, "MISSIONS"), (6, "DEMO"), (7, "NONE"), (8, "CHEAT"), (9, "BLANK"), (10, "DEBUG_SP"),
        (11, "PROMO"), (12, "NAXX"), (13, "GVG"), (14, "BRM"), (15, "TGT"), (16, "CREDITS"),
        (17, "HERO_SKINS"), (18, "TB"), (19, "SLUSH"), (20, "LOE"), (21, "OG"), (22, "OG_RESERVE"),
        (23, "KARA"), (24, "KARA_RESERVE"), (25, "GANGS"), (26, "GANGS_RESERVE"), (27, "UNGORO")
    ],
)

With the following style:

[yapf]
ALIGN_CLOSING_BRACKET_WITH_VISUAL_INDENT = false
ALLOW_MULTILINE_LAMBDAS = false
ALLOW_MULTILINE_DICTIONARY_KEYS = false
BLANK_LINE_BEFORE_NESTED_CLASS_OR_DEF = false
BLANK_LINE_BEFORE_CLASS_DOCSTRING = false
COALESCE_BRACKETS = false
COLUMN_LIMIT = 92
CONTINUATION_INDENT_WIDTH = 1
DEDENT_CLOSING_BRACKETS = true
EACH_DICT_ENTRY_ON_SEPARATE_LINE = true
INDENT_DICTIONARY_VALUE = false
INDENT_WIDTH = 1
JOIN_MULTIPLE_LINES = true
USE_TABS = true
SPACES_AROUND_DEFAULT_OR_NAMED_ASSIGN = false
SPACES_BEFORE_COMMENT = 2

Gets turned into this:

(
    [
        (0, "INVALID"), (1, "TEST_TEMPORARY"), (2, "CORE"), (3, "EXPERT1"), (4, "HOF"),
        (5, "MISSIONS"), (6, "DEMO"), (7, "NONE"), (8, "CHEAT"), (9, "BLANK"), (10, "DEBUG_SP"),
        (11, "PROMO"), (12, "NAXX"), (13, "GVG"), (14, "BRM"), (15, "TGT"), (16, "CREDITS"),
        (17, "HERO_SKINS"), (18, "TB"), (19, "SLUSH"), (20, "LOE"), (21,
                                                                                                                                                                                                                                                            "OG"), (22, "OG_RESERVE"),
        (23, "KARA"), (24, "KARA_RESERVE"), (25, "GANGS"), (26, "GANGS_RESERVE"), (27, "UNGORO")
    ],
)

And yet, if I add a trailing comma at the end of the list of tuples (right after (27, "UNGORO")), it becomes this (which is the intended style):


(
    [
        (0, "INVALID"),
        (1, "TEST_TEMPORARY"),
        (2, "CORE"),
        (3, "EXPERT1"),
        (4, "HOF"),
        (5, "MISSIONS"),
        (6, "DEMO"),
        (7, "NONE"),
        (8, "CHEAT"),
        (9, "BLANK"),
        (10, "DEBUG_SP"),
        (11, "PROMO"),
        (12, "NAXX"),
        (13, "GVG"),
        (14, "BRM"),
        (15, "TGT"),
        (16, "CREDITS"),
        (17, "HERO_SKINS"),
        (18, "TB"),
        (19, "SLUSH"),
        (20, "LOE"),
        (21, "OG"),
        (22, "OG_RESERVE"),
        (23, "KARA"),
        (24, "KARA_RESERVE"),
        (25, "GANGS"),
        (26, "GANGS_RESERVE"),
        (27, "UNGORO"),
    ],
)

Also note that it's much faster. Yapf is super slow at formatting the former; the difference is in several hundreds of ms.

bwendling commented 6 years ago

Did you miss some of the list elements after the (21, in the output above?

As for the time it takes, it's probably simply that the data literal has a lot of elements in it and the number of permutations is large...:-/

jleclanche commented 6 years ago

I didn't miss it, scroll to the side :)

jleclanche commented 6 years ago

The weird thing is that the two tuple representations are syntactically the same, and yapf should be deciding whether or not the tuple should be represented as a vertical list with trailing commas, or inline list without.

bwendling commented 6 years ago

That's ideally what YAPF should be doing, but there's probably some heuristic getting in the way...

jokogr commented 6 years ago

@gwelymernans any idea what's causing it?

The issue is not limited in tuples, I get the same strange behavior with list of lists. E.g.:

% cat test.py
list_of_lists = [
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2]
]
% yapf test.py
list_of_lists = [[1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2],
                 [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2],
                 [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2]]

vs.

% cat test.py
list_of_lists = [
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2],
    [1, 2]
]
% yapf test.py
list_of_lists = [[1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2],
                 [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2],
                 [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1, 2], [1,
                                                                  2], [1, 2]]
bwendling commented 6 years ago

It's definitely a penalty problem. The penalty for splitting between elements of a list is 0 even if the list (or other container) is an element of another list.