Black compromises on commas in nested dict literals? 😱

[dimaqq]

D = {
    "dummy": None,
    **({"has comma": 123,} if some_global else {}),
}
D = {
    "dummy": None,
    **({"no comma?": 123} if some_global else {}),
}

Oddly black is happy with and without trailing comma in the inner dict literal.
And there I thought it was an uncompromising code formatter !;-)

> black t.py
All done! ✨ 🍰 ✨
1 file left unchanged.
> black --version
black, version 19.10b0

[jdufresne]

Bisected to 9854d4b.

[durin42]

From my perspective this is WAI, because black has to preserve trailing commas in order to key collection-exploding behavior off them.

[dimaqq]

From my perspective this is WAI, because black has to preserve trailing commas in order to key collection-exploding behavior off them.

Could you retype or rephrase that? I’m not in the know and I can’t parse the last half...

[durin42]

In my change (#826) the goal was to preserve a collection across multiple lines unconditionally if-and-only-if it has a trailing comma. In order to get that behavior, black had to stop throwing out trailing commas, which it used to do.

[jdufresne]

The collection with the comma, {"has comma": 123,}, isn't split across mutliple lines, though. So why does it keep the comma?

[durin42]

That kind of has two parts to it, so I'm going to write out two questions and answer them:

Q) Why isn't the trailing comma deleted?

A) Because in order to implement #826 we had to stop deleting trailing commas. Black used to do that uncondtionally and then reinsert them if a collection was split onto multiple lines. Now it simply no longer deletes trailing commas.

Q) If that collection has a trailing comma, why isn't it exploded onto multiple lines?

A) Because I couldn't figure out how to make the logic work coherently for "inner" collection literals, the explode-if-trailing-comma logic only applies on the outermost collection literal.

Hopefully that clarifies? In any event I'm not a black maintainer, so I can't speak to whether or not they consider either of these behaviors a defect.

[nguyenbathanh]

I'm not sure if this is related but I'm facing the same issue about a comma left in dictionary.

code.py:

data = {
    'token': jwt.encode({
        'user': '[email protected]',
    })
}

$ black --skip-string-normalization code.py

returns:

data = {'token': jwt.encode({'user': '[email protected]',})}

[Peque]

As @dimaqq pointed out, Black is okay with either formatting, and that should not be the case.

If I had to choose, I would prefer not having a trailing comma when the collection is on a single line (for simplicity and aesthetics).

If I had to be more more objective, then I would say that is still a better choice since it would be flake8 compatible. So why further differ from other widely used linters/formatters?

[LilyFoote]

I think this also applies to tuple unpacking. Black is happy with both:

first_item, second_item, third_item = [1, 2, 3]
(first_item, second_item, third_item,) = [1, 2, 3]

I think black should always use the first.

[liberforce]

Whatever choice you make, the black help will probably need to be updated: https://black.readthedocs.io/en/stable/the_black_code_style.html#trailing-commas

It currently says:

Unnecessary trailing commas are removed if an expression fits in one line. This makes it 1% more likely that your line won’t exceed the allotted line length limit. Moreover, in this scenario, if you added another argument to your call, you’d probably fit it in the same line anyway. That doesn’t make diffs any larger.

[JelleZijlstra]

This is covered by #1288.