black icon indicating copy to clipboard operation
black copied to clipboard

Allow reduced number of blank lines for dummy implementations

Open antonagestam opened this issue 3 years ago • 6 comments

Hi!

I would like to start a discussion about reducing blank lines in some special cases, I hope this is welcome :)

The Blank Lines section in PEP8 states:

Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

While this is in the same paragraph as a sentence about functions, I would like to propose a liberal but well defined interpretation of this. It boils down to allowing reducing blank lines for empty classes and functions. This will make code that uses a lot of type definitions or overloads more readable. In the case of overloads this especially makes sense as it will allow overload definitions to be visually grouped with the overloaded function.

My opinion is also that the requirement for classes to take at least two lines and require being surrounded by two blank lines makes these definition feel heavy. Now talking about how code feels might seem misplaced and unscientific mumbo-jumbo, but I think it is important to allow class definitions to feel light when they're used in a Type-driven development style. Some of these usecases for class definitions that would benefit from feeling lighter are custom exceptions, and intersections. Why should a type definition like A = NewType("A", int) only require one line while a type definition like class Intersection(A, B): ... require six? I argue that making these definitions feel heavy will make developers refrain from using more elegant solutions that utilizes the type system over solutions that feel light but are in reality less expressive and doesn't describe business domains as fully as they could.

  1. Allow putting the entire definition on one line if the entire body consists of pass or ... (ellipses).
class CustomError(Exception): ...

def fn(a): ....
  1. Allow removing blank lines between empty classes and between functions, but require one blank line between an empty class and an empty function. Functions and classes with a docstring as the entire body will also be considered "empty", but not allowed to be defined on a single line.
class CustomError(Exception):
    """Allow documented classes to feel cozy too, as we don't want to discourage docs."""
class OtherError(CustomError): ...

def dummy(a): ...
def other(b): ...
  1. Non-dummy classes and functions should remain their current set of rules for surrounding blank lines. With the exception of overloads. The rule for overloads could be generalized as to allow functions with the same name to skip blank lines between them, but to require two blank lines surrounding them.
class Error(Exception): ...


class Implementation(Exception):
    """I am not an empty class and require some space"""

    a = 1


def fn(): ...


@overload
def a(arg: int) -> int: ...
@overload
def a(arg: str) -> str: ...
@overload
def a(arg: object) -> NoReturn: ...
def a(arg: Union[int, str, object]) -> Union[int, str]:
    if not isinstance(arg, (int, str)):
        raise TypeError
    return arg

While I think this could become well defined, and should before implementation work starts, I'm sure I've missed some edge cases that needs defining. Let's hammer them out if this is welcomed as a good idea in the first place, and not shot down ;)

Current behaviour

Example 2. is currently reformatted by black as (14 lines instead of 6):

class CustomError(Exception):
    """Allow documented classes to feel cozy too, as we don't want to discourage docs."""


class OtherError(CustomError):
    ...


def dummy(a):
    ...


def other(b):
    ...

Example 3. is currently formatted like this with black (33 instead of 22). Notice how information is lost here, the original formatting communicates to a reader that the overloads are related to each other:

class Error(Exception):
    ...


class Implementation(Exception):
    """I am not an empty class and require some space"""

    a = 1


def fn():
    ...


@overload
def a(arg: int) -> int:
    ...


@overload
def a(arg: str) -> str:
    ...


@overload
def a(arg: object) -> NoReturn:
    ...


def a(arg: Union[int, str, object]) -> Union[int, str]:
    if not isinstance(arg, (int, str)):
        raise TypeError
    return arg

antonagestam avatar Oct 30 '20 10:10 antonagestam

For people coming to this issue looking for a solution for stubs, the --pyi flag is what you're looking for. It's designed for typing stubs, telling Black to follow the typeshed formatting conventions instead of Black's own.

ichard26 avatar Nov 25 '20 23:11 ichard26

+1 for keeping related overloads visually together

rhkleijn avatar Feb 02 '21 20:02 rhkleijn

I came here to request exactly this enhancement (point number 3) — in particular, a sequence of @overload-annotated functions in front of a real function are, conceptually, part of the type-annotations of that function, and so it makes sense to group them visually with the function instead of setting them off.

wiml avatar Apr 17 '21 20:04 wiml

Any action on this?

gegnew avatar Feb 17 '22 10:02 gegnew

I'm sympathetic to this change, especially for overloads.

One problem will be that linters like flake8 are going to complain about py files with too few blank lines. We don't guarantee flake8 compatibility, of course, but this will mean people have to turn off a few more flake8 rules.

JelleZijlstra avatar Feb 17 '22 14:02 JelleZijlstra

One problem will be that linters like flake8 are going to complain about py files with too few blank lines. We don't guarantee flake8 compatibility, of course, but this will mean people have to turn off a few more flake8 rules.

Although I also don’t think we should be constrained by Flake8, we could make significant progress here even within that constraint—Flake8 has no complaints about

from typing import NoReturn, Union, overload


def dummy(a): ...
def other(b): ...


@overload
def a(arg: int) -> int: ...
@overload
def a(arg: str) -> str: ...
@overload
def a(arg: object) -> NoReturn: ...


def a(arg: Union[int, str, object]) -> Union[int, str]:
    if not isinstance(arg, (int, str)):
        raise TypeError
    return arg

It still expects the two blank lines between the group of overload declarations and the main definition of the same function, but 2n + 2 lines for n overloads is still much more readable than 5n.

andersk avatar Nov 14 '22 01:11 andersk

I would accept a PR that does the following:

  • If the body of a class or function is just ..., put it on the same line directly after the colon
  • If multiple adjacent functions with the same name each have only ... as their body, remove the blank lines between them. (This would apply to overloads in practice, but avoids hardcoding behavior for the "overload" name.)

JelleZijlstra avatar Apr 29 '23 01:04 JelleZijlstra

Any progress on this?

It's a pretty glaring omission that honestly makes black unusable if you have a lot of overloads.

shayded-exe avatar Jul 05 '23 00:07 shayded-exe

@shayded-exe I think it's safe to assume that progress will be reported in this thread. If you're unhappy with the state of things, there's usually two things you can do to make change in an open source project:

  • Contribute yourself.
  • Pay someone else to contribute.

While I'm too am keen to see this happen, comments just asking for updates are not helpful, and unnecessarily pings lots of people's inboxes.

antonagestam avatar Jul 05 '23 22:07 antonagestam

@antonagestam Sorry, I didn't mean to sound so bitchy (and ping everyone's inboxes three times now).

I really just wanted to express that I'm shocked that more people haven't requested it. Sometimes issues don't get movement unless people are asking for it. I honestly would contribute if I had the time and didn't have other higher priorities.

Anyways, sorry for being annoying :)

shayded-exe avatar Jul 07 '23 00:07 shayded-exe

I understand. that some people like that, but I don't like it to be turned on by default without any option to disable. Additionally, it clashes with Flake8 E701 multiple statements on one line (colon)

eirnym avatar Feb 29 '24 23:02 eirnym

We recommend disabling E701 https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#e701-e704 (and generally most formatting-related lint rules — let Black worry about and fix that for you)

hauntsaninja avatar Mar 01 '24 00:03 hauntsaninja

I don't like such drastic changes enabled by default.

eirnym avatar Mar 01 '24 07:03 eirnym