psf / black

The uncompromising Python code formatter
https://black.readthedocs.io/en/stable/
MIT License
39.21k stars 2.48k forks source link

Strange formatting of fluent interface #571

Open WouldYouKindly opened 6 years ago

WouldYouKindly commented 6 years ago

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)
)
max-sixty commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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
MrkGrgsn commented 4 years ago

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.

vedal commented 3 years ago

Is this possible to define flhent interface in black settings?

peterHoburg commented 3 years ago

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

felix-hilden commented 2 years ago

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 commented 2 years ago

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

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

PeterJCLaw commented 2 years ago

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

felix-hilden commented 2 years ago

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

joshdunnlime commented 1 year ago

Anything here?

joaomacalos commented 1 year ago

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

vmgustavo commented 1 year ago

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

JelleZijlstra commented 1 year ago

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

joaomacalos commented 1 year ago

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

ion-elgreco commented 1 year ago

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

mo2menelzeiny commented 8 months ago

can we disable this with a flag?

leotrs commented 1 day ago

Any comments from a maintainer? This feature would benefit the scientific Python community greatly since pandas encourages method chaining so much.

tijlk commented 10 hours ago

I'm using a lot of PySpark, and the following

retail_accounts_with_segment_df = (
    retail_accounts_df
    .withColumn("kim_segment_cd", f.lit("not applicable"))
    .withColumn("sbi_id", f.lit("not applicable"))
)

annoyingly gets formatted by black as:

retail_accounts_with_segment_df = retail_accounts_df.withColumn(
    "kim_segment_cd", f.lit("not applicable")
).withColumn("sbi_id", f.lit("not applicable"))

which is way less readable. I know you're looking for PR's, @JelleZijlstra , but not everyone is in the open source community business. Comments like this are just the users of these open source packages hoping to influence the prioritisation a little bit.