black icon indicating copy to clipboard operation
black copied to clipboard

Line too long: Lambda with many parameters

Open mikelane opened this issue 6 years ago • 16 comments

Operating system: macos 10.14.4 Python version: 3.6.7 Black version: 19.3b0 Does also happen on master: Yes

We have an interesting way to create pytest fixtures. We've created a function called lambda_fixture() that lets us skip most of the boilerplate issues when creating fixtures. It looks something like this:

my_user = lambda_fixture(lambda user_factory: user_factory(name='Mike'))

That's all well and good, but sometimes these things can have a pretty long list of parameters. Unfortunately, black doesn't handle this very well:

my_long_variable_name = lambda_fixture(
    lambda fixture_1, another_long_fixture_name, some_fixture_factory, some_long_fixture_name_again, short, wow, long_fixture_name: some_fixture_factory.create_batch(
        size=10,
        something=short,
        something_else=long_fixture_name,
    )
)

It would be nice to have black wrap and/or chop down the lambda parameter list. Bonus points if the indent of the wrap/chop is different than the indent of the body of the lambda.

mikelane avatar Apr 18 '19 20:04 mikelane

What formatting do you suggest?

zsol avatar Apr 18 '19 20:04 zsol

What formatting do you suggest?

Good question, I should have suggested something. How about this?

my_long_variable_name = lambda_fixture(
    lambda fixture_1, 
           another_long_fixture_name, 
           some_fixture_factory, 
           some_long_fixture_name_again, 
           short, 
           wow, 
           long_fixture_name: 
        some_fixture_factory.create_batch(
            size=10,
            something=short,
            something_else=long_fixture_name,
        )
)

And like regular function params:

my_long_fixture_name_goes_here = lambda_fixture(
    lambda fixture_names, that_take_up, a_lot_of_space, but_not_too_much:
        code_and_stuff.create_batch(goes='here', like='this')
)

mikelane avatar Apr 18 '19 20:04 mikelane

That would be inconsistent with most of Black's formatting choices because indentation is not a multiple of four. Not that I have a better idea though...

JelleZijlstra avatar Apr 19 '19 03:04 JelleZijlstra

This:

my_long_variable_name = lambda_fixture(
    lambda fixture_1, 
        another_long_fixture_name, 
        some_fixture_factory, 
        some_long_fixture_name_again, 
        short, 
        wow, 
        long_fixture_name: 
            some_fixture_factory.create_batch(
                size=10,
                something=short,
                something_else=long_fixture_name,
            )
)

or even this:

my_long_variable_name = lambda_fixture(
    lambda \
        fixture_1, 
        another_long_fixture_name, 
        some_fixture_factory, 
        some_long_fixture_name_again, 
        short, 
        wow, 
        long_fixture_name: 
            some_fixture_factory.create_batch(
                size=10,
                something=short,
                something_else=long_fixture_name,
            )
)

is definitely better than this:

my_long_variable_name = lambda_fixture(
    lambda fixture_1, another_long_fixture_name, some_fixture_factory, some_long_fixture_name_again, short, wow, long_fixture_name: some_fixture_factory.create_batch(
        size=10,
        something=short,
        something_else=long_fixture_name,
    )
)

advance512 avatar Apr 23 '19 17:04 advance512

This is my idea:

lambda kfjlsdkljfskjfldjsklfd, fdsfsdsdffsdsfd, fsdfsdsfd, sfdfsdsfdsfd,
        dkljsfjlkdsljksdfljksdf, sdfsfdfsfsdsdfsdfdsf, sdfsdffsdfds, sdffsdfdsfds

ghost avatar May 18 '19 03:05 ghost

If the lambda definition exceeds the maximum line length, this please:

my_long_variable_name = lambda_fixture(
    lambda fixture_1, 
    another_long_fixture_name, 
    some_fixture_factory, 
    some_long_fixture_name_again, 
    short, 
    wow, 
    long_fixture_name: 
        some_fixture_factory.create_batch(
            size=10,
            something=short,
            something_else=long_fixture_name,
        )

Thank you.

hzulla avatar Feb 23 '22 10:02 hzulla

(btw, we were stumbling into this when we wanted to deal with the W0640(cell-var-from-loop) linter warning. An easy remedy to avoid lambda-in-a-loop bugs is changing:

lambda: do_something_with(looped_variable)

into

lambda looped_variable=looped_variable: do_something_with(looped_variable)

But once you want to copy a lot of variables inside a loop to a later lambda-call in that fashion, black will trip over the same issue as the original reporter did here.

hzulla avatar Feb 23 '22 10:02 hzulla

Hey there,

I have learned by now that functools.partial is functionally equivalent to the way all of us use lamba in this issue.

However, partial is easier to read and black likes it, too!

So we solved our black formatting problem by using partial instead of lambda.

hzulla avatar Mar 23 '22 10:03 hzulla

I have another suggestion (that may go against black's multi-line statement code style because I don't actively use it):

my_long_variable_name = lambda_fixture(
    lambda
        fixture_1, 
        another_long_fixture_name, 
        some_fixture_factory, 
        some_long_fixture_name_again, 
        short, 
        wow, 
        long_fixture_name,
    : 
        some_fixture_factory.create_batch(
            size=10,
            something=short,
            something_else=long_fixture_name,
        )
)

# has parity with
def function(
    fixture_1, 
    another_long_fixture_name, 
    some_fixture_factory, 
    some_long_fixture_name_again, 
    short, 
    wow, 
    long_fixture_name,
): 
    some_fixture_factory.create_batch(
        size=10,
        something=short,
        something_else=long_fixture_name,
    )

Note that the example relies on the lambda being inside parentheses where backslashes are not required.

FichteFoll avatar Jul 20 '22 08:07 FichteFoll

@FichteFoll's proposed style is a syntax error outside parentheses, so we cannot use it for all lambdas.

JelleZijlstra avatar Jul 20 '22 13:07 JelleZijlstra

Indeed, but I would argue that adding parentheses around a lambda that occurs in assignments (the only place I can think of where a lambda outside of parentheses could reasonably appear) is still worth it over the alternatives and also guarantees that you can have the : delimiter on the same column as the opening lambda keyword.

Probably also noteworthy that pycodestyle annotates this with E131.

It even works with multiple/destructuring assignments, though I'd suggest not to do that.

x, y = (
    lambda
        fixture_1, 
        another_long_fixture_name, 
        some_fixture_factory, 
        some_long_fixture_name_again, 
        short, 
        wow, 
        long_fixture_name, 
    : 
        some_fixture_factory.create_batch(
            size=10,
            something=short,
            something_else=long_fixture_name,
        ),
    1,
)

FichteFoll avatar Jul 20 '22 23:07 FichteFoll

@FichteFoll's proposed style is a syntax error outside parentheses, so we cannot use it for all lambdas.

Could you share an example where a lambda can appear outside of parentheses? I tried a few cases that seem problematic but failed to come up with an example.

Here, the [...] is part of the lambda body, black not use the can_omit_parentheses layout.

a = lambda self,      araa, kkkwargs=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs ), e=1, f=2, g=2: d + [bbb, ccccccc]
a = [bbb, ccccccc] + lambda self,      araa, kkkwargs=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs ), e=1, f=2, g=2: d

Seems to not be valid python (requires parenthesizing the lambda.

Edit: One problematic case:

lambda aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbb: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

Could we solve this by parenthesizing lambdas in this case (whatever makes it work, because it's nonsensical code anyway)

MichaReiser avatar Nov 03 '23 05:11 MichaReiser

A few more thoughts on this.

  • I would prefer to parenthesize the lambda-body if it expands (or parentheses are required) because it helps with readability, but this isn't compatible with Black
  • My preferred option would be to parenthesize parameters if they expand, but Python 3 prohibits parenthesized parameters 😢
  • My second choice is to indent the lambda parameters if they expand. It will require some trickery with comments but helps with readability overall. However, doing so is only possible when the lambda is in a parenthesized context (f.context().node_level().in_parentheses()). The Parameters formatting should use spaces when outside of a parenthesized context.

The second choice introduces a few new incompatibilities with Black, but should fix the case where Ruff produces invalid syntax:

# Input
lambda self, araa, kkkwargs=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs), e=1, f=2, g=2: d

# Formatted
lambda self,
araa,
kkkwargs=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs),
e=1,
f=2,
g=2: d

Some examples of the change from the ecosystem check

     """
     users_filter = list(
         filter(
-            lambda u: u.get("name", "").lower() == user_to_search
-            or u.get("profile", {}).get("display_name", "").lower() == user_to_search
-            or u.get("profile", {}).get("email", "").lower() == user_to_search
-            or u.get("profile", {}).get("real_name", "").lower() == user_to_search,
+            lambda u: (
+                u.get("name", "").lower() == user_to_search
+                or u.get("profile", {}).get("display_name", "").lower() == user_to_search
+                or u.get("profile", {}).get("email", "").lower() == user_to_search
+                or u.get("profile", {}).get("real_name", "").lower() == user_to_search
+            ),
             users_list,
         )
     )
 
         for rows_group in rows_grouped_by_fee.values():
             rows_group.sort(
-                key=lambda x: x["Change"]
-                if same_assets
-                else x["Change"] * price_at_timestamp[x["Coin"]],
+                key=lambda x: (
+                    x["Change"] if same_assets else x["Change"] * price_at_timestamp[x["Coin"]]
+                ),
                 reverse=True,
             )  # noqa: E501
 
         "suffix2": lambda token: token.text[-2:],
         "suffix1": lambda token: token.text[-1:],
         "pos": lambda token: token.data.get(POS_TAG_KEY, None),
-        "pos2": lambda token: token.data.get(POS_TAG_KEY, [])[:2]
-        if POS_TAG_KEY in token.data
-        else None,
+        "pos2": lambda token: (
+            token.data.get(POS_TAG_KEY, [])[:2] if POS_TAG_KEY in token.data else None
+        ),
         "upper": lambda token: token.text.isupper(),
         "digit": lambda token: token.text.isdigit(),
     }

MichaReiser avatar Nov 03 '23 07:11 MichaReiser