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

Allow reduced number of blank lines for dummy implementations #1797

Closed
antonagestam opened this issue Oct 30, 2020 · 13 comments · Fixed by #3796
Closed

Allow reduced number of blank lines for dummy implementations #1797

antonagestam opened this issue Oct 30, 2020 · 13 comments · Fixed by #3796
Labels
F: empty lines Wasting vertical space efficiently. S: accepted The changes in this design / enhancement issue have been accepted and can be implemented T: style What do we want Blackened code to look like?

Comments

@antonagestam
Copy link

antonagestam commented Oct 30, 2020

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 antonagestam added the T: style What do we want Blackened code to look like? label Oct 30, 2020
@ichard26
Copy link
Collaborator

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.

@rhkleijn
Copy link

rhkleijn commented Feb 2, 2021

+1 for keeping related overloads visually together

@wiml
Copy link

wiml commented Apr 17, 2021

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.

@gegnew
Copy link

gegnew commented Feb 17, 2022

Any action on this?

@JelleZijlstra
Copy link
Collaborator

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.

@andersk
Copy link

andersk commented Nov 14, 2022

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.

@JelleZijlstra
Copy link
Collaborator

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 JelleZijlstra added the S: accepted The changes in this design / enhancement issue have been accepted and can be implemented label Apr 29, 2023
@shayded-exe
Copy link

Any progress on this?

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

@antonagestam
Copy link
Author

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

@shayded-exe
Copy link

@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 :)

@eirnym
Copy link

eirnym commented Feb 29, 2024

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)

@hauntsaninja
Copy link
Collaborator

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)

@eirnym
Copy link

eirnym commented Mar 1, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: empty lines Wasting vertical space efficiently. S: accepted The changes in this design / enhancement issue have been accepted and can be implemented T: style What do we want Blackened code to look like?
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants