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

Odd breaking of operators chain #4455

Open
nowtryz opened this issue Sep 11, 2024 · 1 comment
Open

Odd breaking of operators chain #4455

nowtryz opened this issue Sep 11, 2024 · 1 comment
Labels
T: style What do we want Blackened code to look like?

Comments

@nowtryz
Copy link

nowtryz commented Sep 11, 2024

Describe the style change

I found a behavior that seems inconsistant when expressions are chained with operators. I feel like the most readable solution is to break on the operators which to be the chosen solution for normal statements but not in some cases

Examples in the current Black style

Given the following input code:

value = get_value_from_a_kinda_long_function() + previous_values[-1] + max(offset, threshold)

Black breaks the plus signs and produces:

value = (
    get_value_from_a_kinda_long_function()
    + previous_values[-1]
    + max(offset, threshold)
)

However, given the following input code:

value = get_value_from_a_kinda_long_function_that_is_even_longer()  + max(offset, threshold)

class SomeClass:
    def some_method(value):
        return self.another_method(value - max_size) + self.another_method(min_size - value)

def prepare_batch(batch, device, non_blocking):
    x, y = batch["image"].to(device, non_blocking=non_blocking), batch["label"].to(device, non_blocking=non_blocking)

Black produces the following output:

value = get_value_from_a_kinda_long_function_that_is_even_longer() + max(
    offset, threshold
)

class SomeClass:
    def some_method(value):
        return self.another_method(value - max_size) + self.another_method(
            min_size - value
        )

def prepare_batch(batch, device, non_blocking):
    x, y = batch["image"].to(device, non_blocking=non_blocking), batch["label"].to(
        device, non_blocking=non_blocking
    )

Desired style

I would find it more readable, comprehensible and logic to have the following:

value = (
    get_value_from_a_kinda_long_function_that_is_even_longer()
    + max(offset, threshold)
)

class SomeClass:
    def some_method(value):
        return (
            self.another_method(value - max_size)
            + self.another_method(min_size - value)
        )

def prepare_batch(batch, device, non_blocking):
    x, y = (
        batch["image"].to(device, non_blocking=non_blocking),
        batch["label"].to(device, non_blocking=non_blocking)
    )

Additional context

This category of strange (to me) behavior happens in many different cases as I saw it in standard librairies as well like setuptools (or distutils).

@nowtryz nowtryz added the T: style What do we want Blackened code to look like? label Sep 11, 2024
@nowtryz
Copy link
Author

nowtryz commented Sep 17, 2024

Seems related to #4353

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

No branches or pull requests

1 participant