black
black copied to clipboard
psf
Only enforce blank line separation between functions and classes at the same level of nesting
Black insists that function and class definitions should be separated by blank lines. Good rule! However, I think it's slightly overenthusiastic.
For functions at the same level of nesting, this is a good idea:
def foo():
pass
+
def bar():
pass
And it's not just between two functions, but between any start/end of a function and surrounding code:
FOO = 1
+
def foo():
pass
+
BAR = 2
But! This seems unnecessary – and dare I say, ugly and less readable – when the adjacent line of code is on a different nesting level:
if clogged_stream_maker is not None:
async def flipped_clogged_stream_maker():
return reversed(await clogged_stream_maker())
else:
flipped_clogged_stream_maker = None
I claim that the above snippet (source would be better without any blank lines.
The same applies to classes, like, how does this blank line help anything? (source
def test_wrap_non_iobase():
+
class FakeFile:
def close(self): # pragma: no cover
pass
The intuition here is that blank lines are separators, not headers/footers. And you only need separation between items at the same level of nesting. We don't insist on using blank lines to separate a class or def from the beginning or end of the file; similarly, we shouldn't insist on using them to separate a class or def from the beginning or end of a block.
This would also make black's handling of def and class more consistent with its handling of import. Currently black does use the rule I suggest for blank lines after import statements:
# Black insists on a blank line after the import here:
import foo
BAR = 1
# But not here:
try:
import foo
except ImportError:
pass
This annoys me because I have a codebase with lots of functions whose first order of business is to define a nested function:
async def foo(): # black wants to insert a blank line after this line
def bar():
...
return await run_sync_in_worker_thread(bar)
Note that making this change would not introduce churn into existing codebases, because black already preserves blank lines. It would just introduce fewer unnecessary blank lines in the future.
I'm not convinced.
The same applies to classes, like, how does this blank line help anything? Well, I remember a certain gentleman fight really hard back when Black was removing unnecessary vertical whitespace within function bodies ;-)
An inner class definitely fits the description of "logical sections within a function" that PEP 8 talks about:
Use blank lines in functions, sparingly, to indicate logical sections.
I like that Black is doing now because it makes it painfully obvious where scope boundaries are. I agree it's aesthetically unpleasant but it's consistent with module scope which is also code that runs from top to bottom, just like a function body.
I don't care much about the case when an inner def or class happens right at the beginning of a function (although it stops looking weird if you'd have a docstring!). But what I think we need to keep for readability is the blank line after an inner function or class. Again, scope boundaries.
Well, maybe except for the case when the following line has a lesser indentation level than the inner function/class. In other words, yes:
def outer():
def inner():
return False
some = other * code
No:
def outer():
try:
def inner():
return False
except E as e:
...
I don't care much about the case when an inner def or class happens right at the beginning of a function (although it stops looking weird if you'd have a docstring!). But what I think we need to keep for readability is the blank line after an inner function or class. Again, scope boundaries.
Well, maybe except for the case when the following line has a lesser indentation level than the inner function/class.
... Did you just argue yourself around to agreeing with me? :-)
The proposal is exactly to keep the blank line, except in cases where the adjacent code has a lesser indentation.
Oh, I guess there is one case where that's different than what I originally said:
if ...:
...
def ...
According to how I originally phrased this ("different level of indentation") this would not need a blank line; according to how I phrased it now ("lesser level of indentation"), this does need a blank line. I actually meant "lesser" all along, I just forgot that this case existed so didn't realize it mattered which one I said :-). (At the bottom of a function/class this ambiguity can't happen, because only a lesser indentation can close the function/class.)
(Similarly to #449, I want the rule that we'll implement to be trivially explicable to beginners.)
Opening blank line vs. closing blank line
I think it's uncontroversial to say that the opening blank line before an inner class or def could be omitted in all cases where the indentation level of the previous line was lower.
Similarly, with closing blank lines, we could intuit that the same rule applies: if the following line's indentation level is lower than the one of the entire inner class or def, skip the blank line. However, there's more to it.
Inner definition size
My intuition is that omitting blank lines for inner classes and defs is only beneficial if those inner definitions are short. If you have an inner definition (like in a decorator, say) which doesn't fit on one screen, you'd want an additional empty line to make it obvious some block of code ended (and the following return belongs to a different function!).
So this is optimizing for short definitions. But how short? I hate magic numbers.
Inner definition blank lines
This is somewhat related to the previous point. If the inner class or def itself contains blank lines, then skipping the closing blank line will just look weird. In fact, in this case having the symmetrical opening blank line is likely also desirable for clarity.
I don't have a strong intuition for how to format long nested definitions, and I don't feel the intuition that embedding a blank line inside a function somehow forces the function to need surrounding blank lines. I suspect this is getting far enough into weird-edge-case-of-weird-edge-case-land that probably any formatting is going to be somewhat unsatisfying.
In most cases where you can have a nested definition, doesn't black already allow users to insert optional blank lines? Maybe the rule would be: when you have a def or class adjacent to less-indented code, then a blank line is optional; otherwise it's mandatory.
A full example of current behavior which we'd like to standardize:
To be clear, the example above does not demonstrate the behavior we'd like, only lists all cases that I find we need to take into account.
I've done a cursory reading of this issue and related issues. I'm asking because this has been in discussion for almost 4 years:
How does a decision like this get made? Are there any actionable items that are required by the team? Will there be a consensus meeting? Etc.
Looking forward to Black having an opinion on this! (I also don't care which opinion, as long as there is a clear and deterministic one). 👍 Black is an awesome program, keep up the great work!
Thanks! I think this issue still needs some discussion, but there's been related work on blank lines recently in #3035 and #2996.
Personally, I'd like to see them removed. And wrt. ambv's example (updated link that works), directly nested functions and methods have the newlines already removed. To me, the only thing I'd change (in addition to #3035 removing leading space in the def -> space -> some code -> space -> def case) would be the if cases. But, ambv might have a point with this being way better for short definitions.
