E741 should work with lambdas #1106
Conversation
|
woops, good catch! |
pycodestyle.py
Outdated
| if is_lambda_arg and prev_text == ":": | ||
| is_lambda_arg = False |
There was a problem hiding this comment.
this technically isn't precise enough due to lambda default arguments:
>>> x = [1, 2, 3]
>>> y = lambda a=x[1:]: a
>>> y()
[2, 3]There was a problem hiding this comment.
good callout! how bout now?
There was a problem hiding this comment.
Also, along these lines there's a false positive in pycodestyle now of:
def a(x=l[1:5]):
...i fixed it for lambdas in this diff, but we could make an issue / fix it for function defs after?
pycodestyle.py
Outdated
| @@ -1545,7 +1554,7 @@ def ambiguous_identifier(logical_line, tokens): | |||
| ident = text | |||
| pos = start | |||
| # function parameter definitions | |||
| if is_func_def: | |||
| if is_func_def or (is_lambda_def and prev_text in ('lambda', ',')): | |||
There was a problem hiding this comment.
I think this is still not quite right? lambda x=a.I: None
There was a problem hiding this comment.
trying to follow -- that example shouldn't trigger an error for a.I, right? in the current implementation it wouldn't because the prior token isn't lambda or ,
There was a problem hiding this comment.
oh hmmm, then how about lambda *i: None or lambda **i: None (I think this already properly handles lambda *, i: None and lambda /, i: None probably)
pycodestyle.py
Outdated
| next_token = tokens[index + 1] if index < len(tokens) - 1 else None | ||
| next_text = next_token[1] if next_token else None |
There was a problem hiding this comment.
idk if it makes much performance difference here -- but I suspect that these should be inside the if below such that they aren't computed unnecessarily -- pycodestyle's checks are fairly "hot" in terms of execution time
pycodestyle.py
Outdated
| next_token = tokens[index + 1] if index < len(tokens) - 1 else None | ||
| next_text = next_token[1] if next_token else None | ||
| if ( | ||
| next_text and next_text in ':,=)' and | ||
| prev_text in ('lambda', ',', '*', '**', '(') | ||
| ): |
There was a problem hiding this comment.
I would write this as:
if index < len(tokens) - 1 and tokens[index + 1][1] in ... and ...:that way you take advantage of short circuiting and don't do unnecessary work
There was a problem hiding this comment.
oh yeah, that's simpler too. nice
|
Oh nice, thanks so much! 🎉 |
|
2.10.0 now still reports on a construction like |
|
yes, see the linked issues as well as the tests in this PR and the changelog |
|
I probably misunderstood the comment about |
|
the rule is for definitions and declarations, not usages |
|
You mean in the second case |
|
maybe you're not thinking about this the right way def f(x):
l = 1
if l == 2:
print(l)
return l + 5if you only require someone to exclude the lint rule on definitions you'd only have to put |
|
anyway, this is all off topic so I'm going to stop replying |
Closes #959
Closes #935