black icon indicating copy to clipboard operation
black copied to clipboard

Break long lines at dominant semantic features

Open naught101 opened this issue 3 years ago • 5 comments

Black breaks long lines with multiple sets of parentheses at the first set of parentheses that is before the character limit, as far as I can tell. This is a problem when you have lines like this:

    assert nx.number_of_selfloops(G) == len(arch_elements), f"Not all elements have self-loops:\n  {set(arch_elements) - set(nx.selfloops(G))}"

Because black breaks them like this:

    assert nx.number_of_selfloops(G) == len(
        arch_elements
    ), f"Not all elements have self-loops:\n  {set(arch_elements) - set(nx.selfloops(G))}"

Which makes the line much harder to read, because the logic of the assertion is broken onto two lines.

The logical semantic place to break the line is at the , before the f-string, but in python this would require either a \ at the end of the line (ugly AF), or manually parenthesising either the first or last major component, e.g.:

    assert (
        nx.number_of_selfloops(G) == len(arch_elements)
    ), f"Not all elements have self-loops:\n  {set(arch_elements) - set(nx.selfloops(G))}"

or

    assert nx.number_of_selfloops(G) == len(arch_elements), (
        f"Not all elements have self-loops:\n  {set(arch_elements) - set(nx.selfloops(G))}"
    )

Both of these styles are much more readable, the latter being the best to my eyes. Unfortunately if I manually write like this, Black formats it back to the same, broken style. (Note: in this example the longest of my formatted lines is 91 characters, but I have my Black line-length set to 110).

I don't know enough about how Black works internally to be of much help with figuring out a solution, unfortunately. Is it possible that the last set of parentheses in my last example are being removed before the line-length calculation takes place?

naught101 avatar Oct 21 '22 06:10 naught101

If you put brackets around both arguments of the statement i.e.

assert ( nx.number_of_selfloops(G) == len(arch_elements), f"Not all elements have self-loops:\n  {set(arch_elements) set(nx.selfloops(G))}" )

Black outputs

assert (
    nx.number_of_selfloops(G) == len(arch_elements),
    f"Not all elements have self-loops:\n  {set(arch_elements) - set(nx.selfloops(G))}",
 )

charlie572 avatar Oct 28 '22 13:10 charlie572

If you put brackets around both arguments of the statement i.e.

assert ( nx.number_of_selfloops(G) == len(arch_elements), f"Not all elements have self-loops:\n  {set(arch_elements) set(nx.selfloops(G))}" )

Black outputs

assert (
    nx.number_of_selfloops(G) == len(arch_elements),
    f"Not all elements have self-loops:\n  {set(arch_elements) - set(nx.selfloops(G))}",
 )

This is not a solution, it breaks the code. assert (a, b) is assert <tuple> is always true (because a 2-tuple evaluates to true) and something different than assert (a), b or assert a, (b).

e-gebes avatar Oct 31 '22 12:10 e-gebes

Good catch!

Maybe assert will be a function in python 4 :sweat_smile:

naught101 avatar Nov 02 '22 11:11 naught101

Maybe assert will be a function in python 4 😅

unlikely, I would much rather see this fixed much sooner

another example of the issue black will turn

assert isinstance(hitbox["e"], (int, float)), hitbox does not have member e (for east)"

into

 assert isinstance(
     hitbox["e"], (int, float)
 ), "hitbox does not have member e (for east)"

which is significantly harder to read. A possible solution (as mentioned in #1483) would be to format it as either

 assert isinstance(hitbox["e"], (int, float)), (
    "hitbox does not have member e (for east)"
)

or (what it does if there is no parentheses)

assert (
     isinstance(hitbox["e"], (int, float)) 
), "hitbox does not have member e (for east)"

I also request this get the label of bug due to the current formatting being unambiguously worse then no formatting at all

Edit: added 2nd solution

Lincoln-Chamberlin avatar Nov 06 '22 04:11 Lincoln-Chamberlin

Related issue about assert formatting (although not duplicate): #348

felix-hilden avatar Jan 13 '23 20:01 felix-hilden