-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
C2901 & C2902: New checks for unnecessary lambda expression usage #6004
C2901 & C2902: New checks for unnecessary lambda expression usage #6004
Conversation
tests/functional/u/unnecessary/unnecessary_lambda_assignment.py
Outdated
Show resolved
Hide resolved
There were a lot of existing tests using that I had to add ignores to in that last commit, so look at the commit before that for easier reviewing |
3bece9d
to
851bbc8
Compare
Pull Request Test Coverage Report for Build 2269685819
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is hilariously unlikely to happen, but what do you think about attempting to catch this case? (It doesn't emit a warning even if you remove the disable):
https://github.com/PyCQA/pylint/blob/edf753e6864224ac0144c0ac12c8d89baa93e097/tests/functional/a/assignment/assignment_expression.py#L44
@jacobtylerwalls I've extended the check to flag assignment via named expressions as well 👍 |
Yes, look at a .rc files in |
Thanks! |
(sorry about the closing and reopening btw, some of the checks keep timing out for no reason (seems like a GHA thing rather than the code changes and closing/reopening is the easiest way to kick it off again) |
Yes, the python 3.8 + coverage job regularly takes more than 15mn. It's more frequent than an actual failed tests right now. |
@Pierre-Sassoulas Not sure if this is an option, but as a collaborator I can re-run actions. Can we allow this for contributors as well? That might make this more workable. |
I don't think so, but the trick to open/close the issue should do most of the time. |
is there a reason why we don't increase the |
From what I understand the pipeline that are stuck are stuck because the runner used by github has only one core and we try to use two (and we can't ask for multiple core so it's 'random'). So it will fail even if we wait one hour. 15mn permit to free the worker sooner and have overall better availability. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I think it would be a nice check to inaugurate the false positive github action primer when it's available. It's a default checker and it's going to be run in a lot of code once it lands in pylint.
@Pierre-Sassoulas @jpy-git For some reason the latest merge of After that, I think we can go ahead and merge this as we shouldn't let this be blocked by the eventual update to the |
Sure let me fix this, I should have merged that a long time ago. I think the issue is only with the checker number that has been claimed by another new checker since. |
Co-authored-by: Jacob Walls <[email protected]>
Co-authored-by: Jacob Walls <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
for more information, see https://pre-commit.ci
098bbfe
to
eb68655
Compare
@DanielNoord pypy has column/line variation compared to cpython. This is something we do not control and we're probably never going to change the parsing to have the right column in both. Should we still take the column/line into account under PYPY ? |
Let's ignore this test on |
Thanks @jpy-git for contributing the new checker and thanks to all reviewers 🎉 |
Type of Changes
Description
Closes #5976.
Adds new check
unnecessary-lambda-assignment
:Lambda expression assigned to a variable. Define a function using the "def" keyword instead.
From PEP8:
This is equivalent to
E731
in flake8 but additionally addresses several false negatives that are missed by that implementation: PyCQA/pycodestyle#1061Also adds a new check
unnecessary-direct-lambda-call
:Lambda expression called directly. Execute the expression inline instead.
i.e.
a**2
not(lambda x: x**2)(a)
This prevents a weird redundant usage of lambda functions that I've seen some junior devs do before.