Skip to content
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

Break long lines at dominant semantic features #3346

Open
naught101 opened this issue Oct 21, 2022 · 5 comments
Open

Break long lines at dominant semantic features #3346

naught101 opened this issue Oct 21, 2022 · 5 comments
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?

Comments

@naught101
Copy link

naught101 commented Oct 21, 2022

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 naught101 added the T: style What do we want Blackened code to look like? label Oct 21, 2022
@JelleZijlstra JelleZijlstra added the F: linebreak How should we split up lines? label Oct 21, 2022
@charlie572
Copy link

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))}",
 )

@e-gebes
Copy link

e-gebes commented Oct 31, 2022

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).

@naught101
Copy link
Author

Good catch!

Maybe assert will be a function in python 4 😅

@Lincoln-Chamberlin
Copy link

Lincoln-Chamberlin commented Nov 6, 2022

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

@felix-hilden
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

6 participants