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

magic trailing comma is ignored in function return type if function has args #2498

Closed
nik-sm opened this issue Sep 17, 2021 · 4 comments
Closed
Labels
F: trailing comma Full of magic T: style What do we want Blackened code to look like?

Comments

@nik-sm
Copy link

nik-sm commented Sep 17, 2021

Describe the style change

For a function with no args, magic trailing comma lets us control how the return type looks.

However, if we add an arg, the magic trailing comma is ignored, and the return type stays on one huge line.
Instead, it should be split across lines.

Examples in the current Black style

For brevity, suppose all snippets begin with

from typing import Tuple

class Value:
    pass

Consider the case of --line-length 120. Length is not a factor here; in all cases, the entire definition could fit on one line.

With no arguments to def fn(), adding the comma will take us from this:

def fn() -> Tuple[Tuple[Value, Value, Value], Tuple[Value, Value, Value], Tuple[Value, Value, Value]]:
    ...

... to this:

def fn() -> Tuple[
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
]:
    ...

This is nice - the function has an unwieldy return type, and presented on one line, it is not very unreadable.
This multi-line version is also better for a git diff.


Now the if the function has an arg, the magic comma will take us from this:

def fn(arg) -> Tuple[Tuple[Value, Value, Value], Tuple[Value, Value, Value], Tuple[Value, Value, Value]]:
    ...

... to this:

def fn(
    arg,
) -> Tuple[Tuple[Value, Value, Value], Tuple[Value, Value, Value], Tuple[Value, Value, Value],]: 
    ...

Notice the two changes that were made.

  1. arg receives a trailing comma and its own line
  2. The return type's trailing comma is ignored, and it stays on one huge line

This also produces a flake8 error E231 (missing whitespace after ,).

Desired style

I can think of two desired results.

First (preferred):

def fn(arg) -> Tuple[   # single arg, no need for additional newline
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
]:
    ...

Second:

def fn(
    arg,  # Extra newline - not really necessary, but more consistent with example below
) -> Tuple[
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
]:
    ...

This second version is consistent with how the function would look when the return type grows and exceeds line length.
It's not ideal that the arguments are exploded unnecessarily, but it seems like this is possibly a separate topic.

def fn(
    arg,
) -> Tuple[
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],
    Tuple[Value, Value, Value],  # Here it cannot fit on one line
]:
    ...
@nik-sm nik-sm added the T: style What do we want Blackened code to look like? label Sep 17, 2021
@francois-rozet
Copy link

francois-rozet commented Jul 20, 2022

This error is not restricted to function signatures.

cond = [
    1,
] and [
    2,
]

SomeClass(
    a=a,
    b=b,
    c=c,
).method(
    d=d,
    e=e,
)

is formatted to

cond = [1,] and [
    2,
]

SomeClass(a=a, b=b, c=c,).method(
    d=d,
    e=e,
)

which is not what is expected.

@JelleZijlstra JelleZijlstra added the F: trailing comma Full of magic label Jul 20, 2022
@JelleZijlstra
Copy link
Collaborator

I think this is basically #1671.

@JelleZijlstra JelleZijlstra closed this as not planned Won't fix, can't repro, duplicate, stale Jul 20, 2022
@nik-sm
Copy link
Author

nik-sm commented Jul 20, 2022

Thanks for having a look at this.

Is this just closed as a duplicate of #1671?

Whereas #1671 has only the final comma taking effect, this has only the initial comma (inside the function args) taking effect. Not sure, but it seems like these might be different.

@JelleZijlstra
Copy link
Collaborator

I guess it's slightly different, but it's close enough that I'd rather track them both in a single issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: trailing comma Full of magic T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

3 participants