Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E731: Lambda default value for a Callable dataclass field is incorrectly flagged as lambda assignment #10718

Closed
richardxia opened this issue Apr 1, 2024 · 3 comments · Fixed by #10720
Assignees
Labels
rule Implementing or modifying a lint rule

Comments

@richardxia
Copy link

The pycodestyle rule lambda-assignment (E731) is supposed to check whether a lambda expression is being assigned to a variable and instead suggest using a def statement. However, Ruff's implementation appears to be incorrectly flagging a dataclass field with a default value consisting of a lambda expression as violation of this rule, whereas pycodestyle does not. Here is a short example:

from dataclasses import dataclass
from typing import Callable


@dataclass
class Filter:
    filter: Callable[[str], bool] = lambda _: True

Here is the command-line output of both Ruff and pycodestyle:

$ ruff check --isolated --select=E731 mycode.py 
mycode.py:7:5: E731 Do not assign a `lambda` expression, use a `def`
Found 1 error.
$ pycodestyle --select=E731 mycode.py

(No output for pycodestyle, but exits successfully with status code 0.)

The examples above were run with ruff 0.3.4 and pycodestyle 2.11.1, which are the latest versions at the time of this writing.

This should apply to more classes than just dataclasses, but I'm not exactly sure where I would draw the line. You probably do want to flag cases where someone assigned a lambda to a class variable when it should have been a class or instance method, but I'm not sure which heuristics you want to apply.

I haven't tried reading the pycodestyle source code to see how they implemented it, but playing with my example above, I think their heuristic may just be whether or not an annotation is present on the class member declaration, which seems reasonable to me. Here are a few specific example with inline comments on whether pycodestyle reports a violation:

from dataclasses import dataclass
from typing import Callable


# pycodestyle is happy with this
@dataclass
class FilterDataclass:
    filter: Callable[[str], bool] = lambda _: True


# pycodestyle is happy with this
class FilterClassWithCallableAnno:
    filter: Callable[[str], bool] = lambda _: True


# pycodestyle is happy with this
class FilterClassWithAribtraryAnno:
    filter: Foo = lambda _: True


# pycodestyle is _not_ happy with this
# mycode.py:24:5: E731 do not assign a lambda expression, use a def
class FilterClassWithoutAnno:
    filter = lambda _: True
@charliermarsh
Copy link
Member

Ah yeah that's interesting. I think Pyflakes only flags unannotated assignments.

@charliermarsh
Copy link
Member

I could imagine always ignoring these when they're defined within class scopes. But perhaps best to start with dataclasses.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Apr 1, 2024
@charliermarsh charliermarsh self-assigned this Apr 1, 2024
charliermarsh added a commit that referenced this issue Apr 1, 2024
## Summary

An annotated lambda assignment within a class scope is often
intentional. For example, within a dataclass or Pydantic model, these
are treated as fields rather than methods (and so can be passed values
in constructors).

I originally wrote this to special-case dataclasses and Pydantic
models... But was left feeling like we'd see more false positives here
for little gain (an annotated lambda within a `class` is likely
intentional?). Open to opinions, though.

Closes #10718.
@richardxia
Copy link
Author

Thanks for the super fast response and fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants