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

Strange formatting of fluent interface #571

Open
WouldYouKindly opened this issue Oct 17, 2018 · 20 comments
Open

Strange formatting of fluent interface #571

WouldYouKindly opened this issue Oct 17, 2018 · 20 comments
Labels
F: linebreak How should we split up lines? S: needs discussion Needs further hashing out before ready for implementation (on desirability, feasibility, etc.) T: style What do we want Blackened code to look like?

Comments

@WouldYouKindly
Copy link

WouldYouKindly commented Oct 17, 2018

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
Copy link

max-sixty commented Mar 1, 2019

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
Copy link

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
Copy link

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
Copy link

MrkGrgsn commented Mar 10, 2020

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
Copy link

vedal commented Feb 11, 2021

Is this possible to define flhent interface in black settings?

@JelleZijlstra JelleZijlstra added the F: linebreak How should we split up lines? label May 30, 2021
@peterHoburg
Copy link

peterHoburg commented Oct 20, 2021

Related to my comment: #2279 (comment)

@felix-hilden felix-hilden added the S: needs discussion Needs further hashing out before ready for implementation (on desirability, feasibility, etc.) label Jan 31, 2022
@felix-hilden
Copy link
Collaborator

felix-hilden commented Jan 31, 2022

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
Copy link
Collaborator

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.

@PeterJCLaw
Copy link

From #571 (comment) 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

@felix-hilden
Copy link
Collaborator

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

@joshdunnlime
Copy link

Anything here?

@joaomacalos
Copy link

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
Copy link

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

@JelleZijlstra
Copy link
Collaborator

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

@joaomacalos
Copy link

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

@ion-elgreco
Copy link

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

@mo2menelzeiny
Copy link

can we disable this with a flag?

@leotrs
Copy link

leotrs commented Nov 24, 2024

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

@tijlk
Copy link

tijlk commented Nov 25, 2024

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.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 26, 2024

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.

There is no "business" and there is no "prioritisation". There are only PRs. Everyone here is a volunteer.

I'm locking this issue to prevent more unproductive notifications for the many people interested in this issue. PRs are still welcome!

@psf psf locked as off-topic and limited conversation to collaborators Nov 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F: linebreak How should we split up lines? S: needs discussion Needs further hashing out before ready for implementation (on desirability, feasibility, etc.) T: style What do we want Blackened code to look like?
Projects
None yet
Development

Successfully merging a pull request may close this issue.