black: Add linebreak in comprehension with complex element expression

To me Black’s heuristics are too eager to wrap comprehensions up into one line.

Do you think:

(Output from Black)

new_values = [
    left if left < right else right for left, right in values if 0 <= right
]

is easier to skim-read than:

new_values = [
    left
    if left < right else
    right
    for left, right in values
    if 0 <= right
]

I can change the code to have really long names to break Black’s heuristics. And Black outputs a much easier to read format. The only problem is the names I’ve picked to get the nice readable format are way less readable. So now I’ve traded readability for readability.

new_values = [
    a_super_long_name
    if a_super_long_name < b_super_long_name else
    b_super_long_name
    for a_super_long_name, b_super_long_name in values
    if 0 <= b_super_long_name
]

(Output from Black)

new_values = [
    a_super_long_name
    if a_super_long_name < b_super_long_name
    else b_super_long_name
    for a_super_long_name, b_super_long_name in values
    if 0 <= b_super_long_name
]

How can I up the heuristics so much so I can get the following output from Black?

new_values = [
    value
    for value in values
]

I’d much rather always have the above, than have complex comprehensions go on one line.

About this issue

  • Original URL
  • State: open
  • Created 3 years ago
  • Reactions: 7
  • Comments: 16 (1 by maintainers)

Most upvoted comments

Just use # fmt: off and # fmt: on to disable formatting for given code block. Fitting to the single line if possible imho definitely should remain a standard approach.

@Peilonrayz You simply can’t. Black is about compromises driven by the will of the majority / maintainers, not about fitting needs of each individual. You have a different definition of readability than black? So just use yapf and configure it as you wish.

In #2841, a set of criteria for this kind of splitting was proposed. I argued (and presented some examples) that they were too loose, meaning they split too many simple statements, but it could be a good starting point. They were:

  • It has more than one attribute access in any single term of the overall expression
  • It has more than one operator in any single term of the overall expression
  • A single term in the expression contains both an attribute access and an operator
  • The entire expression has a length exceeding 40 chars
  • It has an if clause
  • It has more than 1 for clause

Let’s continue hashing this out!

@felix-hilden Is there still interest in a fix for this issue?

I appreciate that it will be difficult to come up with a good set of heuristics for formatting comprehensions along the lines of those you list. I think this is an important issue for readability, so I’m happy to try to come up with better heuristics if there’s a chance they might get merged.

Alternatively, I have the following thought.

Would it be possible to add a flag to black to stop black from removing line breaks from comprehensions? It seems the problem is always that black collapses lines in comprehensions that were separated for readability.

When such a flag is passed, black would still be permitted to add line breaks, to meet it’s line length requirement and other requirements. To some degree, that flag would provide the best of both worlds, and I think it would make it much easier to use black for those of us with complex comprehensions in our code.

@jaklan Using # fmt: on/off is not a reasonable solution for a couple of reasons.

  1. I want black to format the inner parts of the comprehension.

    Say someone uses ' rather than "; I’d want black to fix that. (I do at times, still converting to ") However the binary nature of # fmt: on/off will mean black won’t fix issues with the code. Additionally we can see me and black have different opinions on how to format the else. When changing to black’s style I will make mistakes inside the # fmt: off which black should fix.

  2. Knowing which comprehensions to # fmt: off is non-obvious.

    I’d not have thought a 7 line comprehension would be wrapped up into one line. As such I’m going to over use # fmt: off and handicap black leading to ¶1.

  3. Converting to black is a needlessly large effort.

    In some projects I have lots of comprehensions. To use black and keep my code readable I would have to manually go through the entire of my code and add # fmt: off everywhere.

  4. Converting my code so black only runs on 50% of the code is a waste.

    If I’m disabling black to the point where 50% of my code isn’t being fixed, then what’s the point in using black at all? I’d have to spend so much time adding # fmt: off and I’d still have to run other tools and manually fix any errors in my comprehensions.

Upping black’s heuristics would fix all of the issues with # fmt: off. Which is why I asked:

How can I up the heuristics so much so I can get the following output from Black?

@awbacker Thanks. Just to be clear, that’s not exactly the suggestion. I’m suggesting that black still be permitted to add linebreaks in a comprehension (list or otherwise) to meet its other requirements, but that it no longer be permitted to remove them.

@nsfinkelstein I like the sound of your suggestions - just don’t edit the newlines in a comprehension outside of function calls embedded within it.

If the temptation to reformat can be overcome, I think this would solve all potential issues:

if expression is ListComprehension:
    raise SkipReformatting()

So please mention people who have your blessing to answer the questions if you don’t want to get them from the community - no need to waste time then.