jsh9 / cercis

https://pypi.org/project/cercis/
MIT License
14 stars 1 forks source link

Allow method chains to have the first method call on a newline #38

Open joshdunnlime opened 8 months ago

joshdunnlime commented 8 months ago

Is your feature request related to a problem? Please describe.

Method chaining is pretty powerful. It is encouraged when using Pandas DataFrames. However, Black regularly reformats this in inconsistent ways:

dataframe_new = (
    dataframe_raw
    .some_function(parameter_1, parameter_2, parameter_3, parameter_4)
    .some_other_func()
    .another_func(
        lambda df: df.long_variable_name + df.another_long_variable_name,
        axis=1,
        kwarg_1=kwarg_1,
    )
)

black does this:

dataframe_new = (
    dataframe_raw.some_function(
        parameter_1, parameter_2, parameter_3, parameter_4
    )
    .some_other_func()
    .another_func(
        lambda df: df.long_variable_name + df.another_long_variable_name,
        axis=1,
        kwarg_1=kwarg_1,
    )
)

It prioritises the first method being on the same line as the original dataframe/class. I think this reduces readability.

Describe the solution you'd like

An option like method-chain-first-method-on-newline which keeps the .some_function( on the second line of the method chain.

Describe alternatives you've considered

Not using Black. The problem here is the rest of the code is horrible to maintain.

Not using Black for blocks of this code. The problem here is the chained methods are rather horrible to manually format.

Using a different formatter. The problem here is most aren't as good as Black, are only minimally configurable or are slow/old/unmaintained etc...

Edge cases

This is fine as this is not yet method chaining:

dataframe_raw.some_function(
        parameter_1, parameter_2, parameter_3, parameter_4
)

This is fine as this is all one line and readable:

dataframe_raw.some_function().sum().mul(3).diff()
char101 commented 8 months ago

It is formatted that way because of https://github.com/psf/black/issues/67#issuecomment-389681164

I use polars and I prefer the more concise formatting, i.e. without that parantheses wrapping

df = pl.DataFrame(
    ...
).with_columns(
    ...
).select(
    ...
)

For this kind of formatting, the patch seems to be easy

diff --git a/src/cercis/lines.py b/src/cercis/lines.py
index e774d63..7efde52 100644
--- a/src/cercis/lines.py
+++ b/src/cercis/lines.py
@@ -1087,7 +1088,7 @@ def can_omit_invisible_parens(

     max_priority = bt.max_delimiter_priority()
     delimiter_count = bt.delimiter_count_with_priority(max_priority)
-    if delimiter_count > 1:
+    if delimiter_count > 100:
         # With more than one delimiter of a kind the optional parentheses read better.
         return False

Normally after 2 method chain, it will be wrapped into parantheses, this changes make it basically to never wrap it in parantheses.

joshdunnlime commented 7 months ago

Thanks for the link. I remember looking at that feed a while ago. It goes against Black's super strict ethos to have it as an option, which is fine, but I raised it here hopefully so that it can be aded as an option.

Personally, I think your method chaining approach is great for shorter chains (in a page long chain you might miss the exist at first glance - but that's pretty minor). I assume the logic is more or less the same - an option for method_chain_dot_on_newline=True/False would be AMAZING