black icon indicating copy to clipboard operation
black copied to clipboard

Strange formatting of fluent interface

Open WouldYouKindly opened this issue 6 years ago • 20 comments

Operating system: Mac Python version: 3.7 Black version: 18.9b0 Does also happen on master:

Following on https://twitter.com/llanga/status/1052631100290420736

I have a code which uses yarl.URL

a = str(
    my_very_very_very_long_url_name
    .with_user(and_very_long_username)
    .with_password(and_very_long_password)
)

Black formats it like this

a = str(
    my_very_very_very_long_url_name.with_user(and_very_long_username).with_password(
        and_very_long_password
    )
)

I understand that first call is not on a new line because of reasons, but the rest still looks quite odd, especially considering that when there are three calls in the chain, Black produces

a = str(
    my_very_very_very_long_url_name.with_user(and_very_long_username)
    .with_user(and_very_long_username)
    .with_password(and_very_long_password)
)

WouldYouKindly avatar Oct 17 '18 19:10 WouldYouKindly

Forgive me for using this as the fluent questions thread, but another case:

With a single call


def x():
    base_ds["security_score_filtered"] = base_ds["security_score_risk_adj"].where(
        base_ds["is_share_price_valid"]
        & base_ds["is_market_cap_valid"]
        & base_ds["is_liquid"]
    )

With two calls


def x():
    base_ds["security_score_filtered"] = (
        base_ds["security_score_risk_adj"]
        .where(
            base_ds["is_share_price_valid"]
            & base_ds["is_market_cap_valid"]
            & base_ds["is_liquid"]
        )
        .sum()  # <- new line here (only change)
    )

Why the switch of the first line between the two?

I would generally prefer the second format even without the second call (though the question stands independent of my opinion)

max-sixty avatar Mar 01 '19 21:03 max-sixty

Adding another here, as a (hopefully) helpful additional case:

From:

    df = (
        gbq
        .read_gbq(query, project="sixty-capital-prod")
        .sort_values("factset_entity_id")
    )

To:

    df = gbq.read_gbq(query, project="sixty-capital-prod").sort_values(
        "factset_entity_id"
    )

max-sixty avatar Mar 21 '19 22:03 max-sixty

And another:

Looks good:

                completion_time = (
                    a.read_namespaced_job(job.metadata.name, namespace="default")
                    .status()
                    .completion_time()
                )

Now without the () on the last two lines after status & completion_time :

                completion_time = a.read_namespaced_job(
                    job.metadata.name, namespace="default"
                ).status.completion_time

max-sixty avatar May 30 '19 23:05 max-sixty

I would prefer that black applies the fluent style across more cases of chained calls and attribute accesses, such as those examples provided above, even if it does use a few more lines. Consistency between these similar multi-line expressions is more valuable to me than removing a few extra lines.

MrkGrgsn avatar Mar 10 '20 00:03 MrkGrgsn

Is this possible to define flhent interface in black settings?

vedal avatar Feb 11 '21 11:02 vedal

Related to my comment: https://github.com/psf/black/issues/2279#issuecomment-948033626

peterHoburg avatar Oct 20 '21 21:10 peterHoburg

Since there are no maintainer comments yet on this issue, I'll echo the points already raised: I'd also like for Black to format fluent interfaces with or without initial parens consistently:

def func():
    (
        thing("argument-1")
        .thing("argument-2")
        .thing("argument-3")
        .thing("argument-4")
        .thing("argument-5")
        .thing("argument-6")
        .thing("argument-7")
        .thing("argument-8")
        .thing("argument-9")
    )

adding the parens rather than splitting a single call if the line is too long.

felix-hilden avatar Jan 31 '22 12:01 felix-hilden

Here's a more complete set of variations I'd expect to behave similarly: playground

  • Regardless of the presence of initial surrounding parentheses, if the line is too long and there is a call chain we should add parens and split on method/attribute access (dots)
  • Whether or not the expression is an assignment should have no effect on the call chain formatting
  • Whether or not the calls contain any arguments should also not have an effect on the formatting beyond splitting further if the call itself is too long, unless:
  • Having magic trailing commas should explode the calls as normal

In the example playground, I find that the cases with no assignment and no initial parens are formatted against my expectations.

felix-hilden avatar Jan 31 '22 22:01 felix-hilden

From https://github.com/psf/black/issues/571#issuecomment-497521878 I suggest we add to the list:

  • Whether or not a member access is a call should have no effect on the call chain formatting

PeterJCLaw avatar Jan 31 '22 22:01 PeterJCLaw

And in that case, #510 is very relevant, if not duplicate. But I'll leave it open for now.

felix-hilden avatar Jan 31 '22 22:01 felix-hilden

Anything here?

joshdunnlime avatar Mar 18 '23 06:03 joshdunnlime

It would indeed be much appreciated if the formatting of fluent interfaces were more consistent. The examples posted by @max-sixty above are very descriptive of the issue.

With method chaining is often better for readability to have more lines but every method in the chain being called in a new line

joaomacalos avatar May 01 '23 15:05 joaomacalos

anything here? this is currently the only thing that is holding me back from using black

vmgustavo avatar Jul 31 '23 19:07 vmgustavo

Please write a PR to fix this issue instead of sending unproductive comments.

JelleZijlstra avatar Jul 31 '23 23:07 JelleZijlstra

@vmgustavo I'm using # fmt : skip after the closing parenthesis of my fluent chains to keep my formatting when black breaks it.

joaomacalos avatar Aug 02 '23 15:08 joaomacalos

@JelleZijlstra is it possible to provide custom method chains? So, this issue gets avoided, or is skipping format the only option here?

ion-elgreco avatar Aug 10 '23 13:08 ion-elgreco

can we disable this with a flag?

mo2menelzeiny avatar Mar 26 '24 23:03 mo2menelzeiny