Feedback on fluent formatting of method chain preview style

[dylwil3]

This discussion is for getting community feedback on the preview formatting style for method chains.

This style formats method chains like so:

--- stable style
+++ preview

 x = (
-    df.filter(at_this_very_long > boolean_condition)
+    df
+    .filter(at_this_very_long > boolean_condition)
     .agg(using_this_long_function_name)
     .merge(other)
 )

While much of the feedback has been positive, the main hesitation has been around the case where the chain begins with top-level methods called from popular libraries such as numpy, pandas, and polars. These result in situations like the following:

--- stable style
+++ preview

 x = (
-    pd.read_csv("my_file_path.csv")
+    pd
+    .read_csv("my_file_path.csv")
     .filter(at_this_very_long > boolean_condition)
     .agg(using_this_long_function_name)
     .merge(other)
 )

 x = (
-    np.arange(shape)
+    np
+    .arange(shape)
     .foooooooooooooooooooooo()
     .barrrrrrrrrrrrrrrrrrr()
     .baaaaaaazzzzzzzzzzz()
 )

Speaking only for myself, I am totally fine with this formatting! However, others may disagree, and I can see how this goes somewhat beyond the intended semantics of "object of interest on one line, followed by transformations on successive lines".

For those proposing alternatives, please note that the formatter does not have access to semantic information (e.g. whether some name is bound to a module), so any proposed heuristics must be syntactical or configuration-based.

Check out the documentation for this style for a detailed description of the style, the original issue for some historical context, and the PR implementing this feature for some additional discussion.

Very much looking forward to your feedback!

[dylwil3]

Please upvote this comment if you like the preview style as-is.

[mcrumiller]

@alexreinking it sounds like you are upvoting/commenting on the wrong section in this case; your "I would have most liked to read" is exactly what you'd get if fluent layout were not stabilized.

That said, I really hope this becomes a configuration option.

[alexreinking]

@mcrumiller -- no. No existing style gives me the "most liked to read" result. Read my comment again.

[dylwil3]

For what it's worth that particular example looks like a bug to me - I'll have to dig into the logic we use to determine whether to apply fluent formatting to see what's going on there.

[mcrumiller]

@mcrumiller -- no. No existing style gives me the "most liked to read" result. Read my comment again.

You're right, my bad.

[tjweitzel225]

I'm in this camp. I'd slightly prefer if exceptionally short names (or specific strings defined in config as suggested above, defaulting to the usual suspects np, pd, sp, plt etc) were handled differently, but overall the preview style is a win.

[dylwil3]

Please upvote this comment if you would not upgrade your Ruff version if it stabilized this preview style.

[mcrumiller]

Upvoted, I strongly dislike this. I encountered this in a pl.DataFrame(...) chain and it was quite aversive to see the pl on a separate line, enough that I probably would start calling from polars import DataFrame, ... to avoid having to write pl at all. The first line in the code chain should be your starting object, which is a DataFrame, not the module that owns it.

[mathisdrn]

The new style formatting is a nice idea but IMO not disambiguating between method chaining on a class instance vs library method call makes it a dealbreaker. This simply does not look right to me:

coverage = (
    pl
    .col("Prediction Set")
    .list.contains(pl.col("True Class"))
    .mean()
    .mul(100)
    .round(2)
    .alias("Coverage")
)

Also with altair chart :

alt
.Chart(corr2, mark=mark, title=f"{corr_type.capitalize()} correlations") 
.transform_calculate(abs_value="abs(datum.value)")
.encode(
    alt.X("index", sort=ind_sort, title=""),
    alt.Y("variable", sort=var_sort[::-1], 

[gusutabopb]

This change is terrible for Polars. In long .select() or .with_columns() calls, pretty much every argument starts with pl.col, so this formatting change adds a lot of of pointless lines with nothing but pl and whitespace.

[alexander-beedie]

As a reference point we've been big fans of the --preview style for the "hugging parentheses" formatting at work, but when the fluent formatting landed in its current state I ended up having to put an upper bound on ruff for now as it was leading to an excessive number of trivial line breaks that overall seemed to make things less readable (I am also a Polars dev, which -given other comments- seems to be particularly affected by this change, but I'm speaking with my "work" hat on here 😅)

I've tried to isolate the cases that "feel" bad into some heuristics that could help, as the idea behind the fluent formatting is definitely a good one! It just feels like it may be a bit over-tuned at the moment.

Possible tweaks:

• "Don't immediately split on leading short/trivial prefixes, eg: pl,pd,np, etc"

  These seem to come up a lot in the comments; maybe don't split unless the prefix is more than 5 chars (for example). Where exactly to set the cutoff might be slightly contentious, but creating an entire new line for these 1-2 letter prefixes generally seems a bit much (the next idea can solve some of the same issues without needing an arbitrary cutoff). Or, as suggested above by @skellys, allow prefixes you don't want split to be specified in the ruff config settings - then the user has more control, and ruff doesn't have to choose an arbitrary cutoff (and/or make the prefix length for splitting a config setting?)

  My understanding is that the formatter doesn't have any knowledge of what modules were imported (and how)? If it did, it could assemble a list of those imports such as import numpy as np, import polars as pl, etc and be aware that "np" and "pl" should not be trivially split) 🤔

• "Don't immediately split if the object/method has arguments that are going to be on new lines anyway"

  To illustrate - as xx.SomeObject arguments are going on a new line anyway, the extra new line that separates "xx" from "SomeObject" arguably hurts clarity because now you have to look up/down to see what kind of "SomeObject" it is. In the case of DataFrame, for example, it could be pandas ("pd") or polars ("pl"), and the difference between the two is very meaningful - you really want that information right next to the object, not on another line:

  # as init params are also going on new lines, the split feels unnecessary
  res = (
      xx
      .SomeObject(
          data=data,
          param=value,
          other_stuff=misc,
      )
      .method(arg=True)
      .finalise()
  )
  
  # keeps the 'xx' next to SomeObject (which is often meaningful/helpful)
  res = (
      xx.SomeObject(
          data=data,
          param=value,
          other_stuff=misc,
      )
      .method(arg=True)
      .finalise()
  )

• And/or... make it one of the (few) optional settings? :)

Side-note: could we have a feature allowing for preview features to be turned on/off individually? Then we can test them in isolation, all at once, or opt to enable/disable specific features without having to go all-in on everything being previewed.

[JWCS]

On the theme of alexander-beedie 's observations, I find that a lot of low-priority "initialization" happens on the first line. When the first line expands, it becomes noise, and hard to tell what's important.

res = MyType(that.api.get_function_call(boilerplate_passthrough = db, boring_arg = "that")
    .transform(...) # The interesting lines we care about start here
    .filter(...)
    ....
)
res = MyType(
    that.api
    .get_function_call(
        boilerplate_passthrough = db,
        boring_arg = "that"
    )
    .transform(...)
    .filter(...)
    ....
)

(Unrelevant gripe: the MyType() cast at the end becoming +1 new line, since you can't function-chain a type coercion).
(Caveat: we're updating to latest ruff, which seems to have fixed the #fmt: skip not actually skipping the whole line and being half useless. In this case, we could just litter fmt skip on low-signal lines. But excessive line expansion on everything, even if it's not reaching EOL limits, is painful).