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

Formatter undocumented deviation: multi assignment wrapping #7317

Closed
JonathanPlasse opened this issue Sep 12, 2023 · 5 comments · Fixed by #7679
Closed

Formatter undocumented deviation: multi assignment wrapping #7317

JonathanPlasse opened this issue Sep 12, 2023 · 5 comments · Fixed by #7679
Assignees
Labels
documentation Improvements or additions to documentation formatter Related to the formatter

Comments

@JonathanPlasse
Copy link
Contributor

Black formatting

def from_user_altitude_range_to_px4_altitude_range(
    altitude_range: Tuple[float, float],
) -> Tuple[int, int]:
    user_minimal_coordinate, user_maximal_coordinate = (0.0, 0.0, altitude_range[0]), (
        0.0,
        0.0,
        altitude_range[1],
    )

Ruff formatting

def from_user_altitude_range_to_px4_altitude_range(
    altitude_range: Tuple[float, float],
) -> Tuple[int, int]:
    user_minimal_coordinate, user_maximal_coordinate = (
        (0.0, 0.0, altitude_range[0]),
        (
            0.0,
            0.0,
            altitude_range[1],
        ),
    )

Use Ruff 0.0.289 with line length 100.

@zanieb
Copy link
Member

zanieb commented Sep 12, 2023

For what it's worth, I think the Ruff formatting is more readable in this example.

@JonathanPlasse
Copy link
Contributor Author

JonathanPlasse commented Sep 12, 2023

I am reporting all deviations I come across that are not documented.
I agree with you Ruff formatting is more readable to me.

@charliermarsh
Copy link
Member

Thanks @JonathanPlasse! That’s great and the intended workflow. We’ll collect undocumented deviations, then make decisions on whether to address them or not.

@charliermarsh charliermarsh added formatter Related to the formatter needs-decision Awaiting a decision from a maintainer labels Sep 12, 2023
@MichaReiser
Copy link
Member

Thanks for reporting this deviation. This is an intentional deviation that I'm open to discussing and that we forgot to document.

Ruff parenthesizes tuples except in a few places, whereas Black strips the parentheses in many places.

# Black
def from_user_altitude_range_to_px4_altitude_range():
    user_minimal_coordinate, user_maximal_coordinate = (0.0, 0.0, altitude_range[0]), (
        0.0,
        0.0,
        altitude_range[1],
    )

    (0.0, 0.0, altitude_range[0]), (
        0.0,
        0.0,
        altitude_range[1],
    )

    assertEquals(0.0, 0.0, altitude_range[0]),

# Ruff
def from_user_altitude_range_to_px4_altitude_range():
    user_minimal_coordinate, user_maximal_coordinate = (
        (0.0, 0.0, altitude_range[0]),
        (
            0.0,
            0.0,
            altitude_range[1],
        ),
    )

    (
        (0.0, 0.0, altitude_range[0]),
        (
            0.0,
            0.0,
            altitude_range[1],
        ),
    )

    (assertEquals(0.0, 0.0, altitude_range[0]),)

Our reasoning behind requiring the parentheses it otherwise can be very hard to spot that it is a tuple. For example, would you have noticed that the assertEquals(0.0, 0.0, altitude_range[0]), is a "useless" tuple? I did not and neither did some contributors to the django project django/django#17181. That's why I propose keeping Ruff's formatting but I'm interested in getting more feedback on this (ideally from other projects trying ruff, are there places where this formatting looks worse?)

@MichaReiser MichaReiser added this to the Formatter: Beta milestone Sep 13, 2023
@MichaReiser MichaReiser removed the needs-decision Awaiting a decision from a maintainer label Sep 27, 2023
@charliermarsh
Copy link
Member

We're considering this an intentional deviation. Can be closed once it's documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation formatter Related to the formatter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants