E731 should not apply when assigning to a field

[mypalmike]

For example, this code is commonly used for setting the http method in a urllib2 request:

request.get_method = lambda: 'DELETE'

The library is expecting a function, and using a lambda here is not against the PEP8 intent as far as I can tell, which is binding a lambda to a name. This could be done with a def, but the code here is simpler (inasmuch as people find lambda to be simple).

[sigmavirus24]

What version of pep8 are you using @mypalmike? I cannot reproduce this using 1.5.7.

[mypalmike]

I believe lambda checking was added fairly recently to the tool. I am using version 1.6.0a0 (ships with pydev/eclipse).

[sigmavirus24]

Here's the thing though, just because it is a common pattern, doesn't mean it is a good pattern. Here's what the PEP 8 has to say about this:

Always use a def statement instead of an assignment statement that binds a lambda expression directly to a name.

E731 is a good addition in my opinion given that. You can easily ignore it (since it seems very specifically targetted to assigning lambda's) with --ignore E731.

[mypalmike]

I agree that E731 is a good addition.

As you mentioned, PEP 8 says, "Always use a def statement instead of an assignment statement that binds a lambda expression directly to a name." The intent appears to be that naming is better accomplished by def than by lambda.

In the case I am describing, the code is not naming a function, but rather binding an anonymous function to a field. Unlike the example in PEP8, I can't change:

a.b = lambda: "VALUE"

to

def a.b():
  return "VALUE"

... which is syntactically incorrect. The other valid approach would require defining a named function and then setting the field to that function name to achieve the same effect, but it's not clear to me that this is the intent of PEP 8. If PEP 8 is intending to state that lambdas should only be used as arguments to functions, you would think it would be worded in that way.

Given the ambiguity of the PEP 8 rule, I'm ok with this being closed. I suspect this will start to bite tkinter and other users of libraries where callbacks are heavily used.

[sigmavirus24]

The language is actually quite clear to me. It says "instead of an assignment statement that binds a lambda expression directly to a name". You're using an assignment statement (a.b =) and you're binding the lambda to a name. The example there is the simplest one, not the all-encompassing one. I am going to ask some people who know better the intention.

[dstufft]

I'm not a PEP8 author/expert but it seems weird to me to be expected to use a def statement for something where you can't actually use a def statement.

[ncoghlan]

It's specifically the case where the assignment statement is equivalent to the one line def statement that the PEP is aiming to provide guidance on. Attribute and item references are not names in that context - it's only referring to standalone identifiers.

[sigmavirus24]

I'll work on a patch to fix this then for later tonight. Thanks for the insight @ncoghlan.

[sigmavirus24]

@mypalmike 397719c should fix this for you. Thanks for reporting it.

[ncoghlan]

FWIW, I just pushed a tweak to PEP 8 that changes the relevant point from "a name" to "an identifier". Hopefully that's clearer for folks that haven't spent way too much time staring at Python's Grammar file :)

[sigmavirus24]

Thanks Nick!

[mypalmike]

Thanks, this definitely clears things up. And the patch addresses the issue for me.

I was thinking of some other cases which might trigger this.

foo['bar'] = lambda: 'x' # I haven't see this, but could be used to dynamically build jump tables.
lambda: 'foo' # Pointless no-op, probably bad, but not specifically violating this PEP 8 rule.

Something like the following might address all cases directly based on ncoghlan's clarified PEP 8 wording:

if before.split('=')[0].strip().isidentifier():

[sigmavirus24]

@mypalmike any feedback on the latest revision to the PR?

[mypalmike]

LGTM. Thanks for your work on this excellent tool.

[sigmavirus24]

Thanks for reporting this bug!

[kartynnik]

What about the legitimate case

return lambda: whatever

?
This is useful in various forms of currying and binding like

def _bind(self, func):
     '''Takes an unbound method and binds it to :py:var:`self`.'''
     return lambda *args, **kwargs: func(self, *args, **kwargs)

It is obviously not a direct assignment to an identifier and still produces E731 for me.

[sigmavirus24]

That example can be fixed by using functools.partial

[jayvdb]

While functools.partial might be a workaround in some cases, that isnt the issue. Using return does not bind the lambda to an identifier.

[kartynnik]

@jayvdb, exactly. @sigmavirus24, it's a workaround for the pep8 behavior, not a fix, because there's nothing violating PEP8 there (we're not binding the lambda to an identifier and we can't rephrase that with a def of the same identifier - we have to think of an intermediate name). Actually, this workaround is not universal either: one might want to return not a partial application of a function, but a completely fresh function that is a closure over the locals; using a nested def looks times uglier and confuses structure folding in some IDEs.

[kartynnik]

I would rephrase the point of PEP8 like this: lambdas are for unnamed functions and it's pointless to use them when you actually need and provide a name via a direct assignment to an identifier (which is really more readable when phrased as a def).