make line wrapping better for inline if statements with comments in function args

[ntextreme3]

I looked through some issues and didn't see any like this. Sorry if this is a duplicate or already discussed. I noticed something I thought was strange and probably the first thing I didn't like about black formatted code 😄

Something like this:

some_kind_of_data = "foo"
some_other_kind_of_data = "bar"
print(
    dict(
        a=1,
        b=2 if some_kind_of_data is not None else some_other_kind_of_data,  # some explanation of why this is actually necessary
        c=3,
    )
)

Using black (python3.7.3, black 19.10b0), becomes:

some_kind_of_data = "foo"
some_other_kind_of_data = "bar"
print(
    dict(
        a=1,
        b=2
        if some_kind_of_data is not None 
        else some_other_kind_of_data,  # some explanation of why this is actually necessary
        c=3,
    )
)

Desired style How do you think Black should format the above snippets:

Either leave it obnoxiously long, or add parenthesis / indentation

some_kind_of_data = "foo"
some_other_kind_of_data = "bar"
print(
    dict(
        a=1,
        b=(
            2 if some_kind_of_data is not None else some_other_kind_of_data
        ),  # some explanation of why this is actually necessary
        c=3,
    )
)

[don-mums]

I have similar example and it seams to be the same core issue. When I have too long chained method calls in our code it looks really weird. This is especially problematic with Django ORM which can chain methods one after another.

Original:

data.update(
    {
        'customer': self.customer,
        'invoices': Invoice.objects.with_prices().with_item_data().select_related('customer', 'customer__billing_address', 'customer__delivery_address').filter(customer=self.customer, date_cancelled__isnull=True),
        'qr': self.get_qr(),
    }
)

Formatted code:

data.update(
    {
        'customer': self.customer,
        'invoices': Invoice.objects.with_prices()
        .with_item_data()
        .select_related(
            'customer', 'customer__billing_address', 'customer__delivery_address'
        )
        .filter(customer=self.customer, date_cancelled__isnull=True),
        'qr': self.get_qr(),
    }
)

What I with my colleagues wish for:

data.update(
    {
        'customer': self.customer,
        'invoices': Invoice.objects.with_prices()
            .with_item_data()
            .select_related(
                'customer', 'customer__billing_address', 'customer__delivery_address'
            )
            .filter(customer=self.customer, date_cancelled__isnull=True),
        'qr': self.get_qr(),
    }
)

The problem is the indentation. I cannot see at first glance, that those other lines are not another keys in dictionary but actual method calls. I know color highlighting might might save this. But I still think that it is not very good idea. Different indentation would be much cleaner.

[ocehugo]

Yeap - I got an even simpler case of black creating ugly code because of long comments.

def create_file():
    with open('z', 'w') as file:  # this comment leaks over the linelimit edge because I'm lazy to break it, or move the comment line somewhere else, or use line splitted """ kind of comments
        file.write('foo')

results in

def create_file():
    with open(
        "z", "w"
    ) as file:  # this comment leaks over the line limit edge because I'm lazy to break it, or move the comment line somewhere else, or use line split """ kind of comments.
        file.write("foo")

None of the above is acceptable. At the bare minimum, the comment line needs to be at its own line given the length.

Black could do any of the four options below:

1. Put the comment above the code line (or below).
2. Break the comment line into several multi # comment.
3. break the comment line into a mult """ comment.

I think 1 is the most plausible thing to do. Black should only allow inline comments when comment respects the limit.

[sebastian-fredriksson-bernholtz]

I also dislike the way that Black handles long inline comments. In my case it reformats (some indentation and other code removed)

# explanation about what the goal of this section of code is (maybe should refactor to its own function)
while (retries < len(RETRY_BACKOFF)):  # loop has an `if: break` when there are no more transactions, and updates offset with batch size on each loop
    other code
    logger.error(error, exc_info=True)  # Only error level since we have retries. If retries fails it's critical.

to

# explanation about what the goal of this section of code is doing (maybe should refactor to its own function)
while retries < len(
    RETRY_BACKOFF
 ):  # loop has an `if: break` when there are no more transactions, and updates offset with batch size on each loop
    other code
    logger.error(
        error, exc_info=True
    )  # Only error level since we have retries. If retries fails it's critical.

only because of the inline comment.

Unlike @ocehugo (unless this is the missing fourth option 😉) I want my formatter to disregard inline comments. I use inline comments to give extra context information about a line of code without being obtrusive. I want to focus on the code, and only look to the comment if I have a question about the code. I think a separate line comment is much more obtrusive. I barely look at the inline comments unless I have a question about the actual line of code.

Since there is no inline exclude (#451) - I would have preferred to use that - I used

# explanation about what the goal of this section of code is doing (maybe should refactor to its own function)
# loop has an `if: break` when there are no more transactions, and updates offset with batch size on each loop
while (retries < len(RETRY_BACKOFF)):  
    other code
    # Only error level since we have retries. If retries fails it's critical.
    logger.error(error, exc_info=True)

But I don't like it that I have to be so careful with inline comments for the risk of creating the mess it does. If I wanted separate line comments instead of inline comments, I would use that.

I also tried using #fmt: off/on but I can't get it to work properly around the while statement (and I think it's more obtrusive than the above anyways)

# explanation about what the goal of this section of code is doing (maybe should refactor to its own function)
# fmt: off
while (retries < len(RETRY_BACKOFF)):  # loop has an `if: break` when there are no more transactions, and updates offset with batch size on each loop
# fmt: on
    other code
    # fmt: off
    logger.error(error, exc_info=True)  # Only error level since we have retries. If retries fails it's critical.
    # fmt: on

[MeGaGiGaGon]

The initial issue was fixed as part of the 12-11-2023 release, most likely as part of #4010. The code now formats to the desired style with parenthesis playground link. I'll be opening a PR to add an anti-regression test case.

@don-mums If you'd like to champion your proposed changes please open a new issue, as this style is intentional, see Call chains

@ocehugo @sebastian-fredriksson-bernholtz If you'd like to continue discussing how Black handles inline comments, there is most likely an existing issue for it, ie #1125. Also see this comment on #3450 for why breaking up/ignoring comments has been rejected in the past.