black icon indicating copy to clipboard operation
black copied to clipboard

Odd breaking of operators chain

Open nowtryz opened this issue 1 year ago • 1 comments

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 avatar Sep 11 '24 20:09 nowtryz

Seems related to #4353

nowtryz avatar Sep 17 '24 07:09 nowtryz