-
-
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
Fix a false positive for simplify-boolean-expression
when multiple values inferred
#7627
Fix a false positive for simplify-boolean-expression
when multiple values inferred
#7627
Conversation
…values inferred Adds a keyword-only `compare_constants` argument to `safe_infer`.
Pull Request Test Coverage Report for Build 3260078556
💛 - Coveralls |
This comment has been minimized.
This comment has been minimized.
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.
Looks good to me! Left one suggestion. Requesting another approver.
I wonder if there's anywhere else compare_constants=True
could be used but after perusing usages of safe_infer
no other applications are immediately clear.
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.
We could re-add the old test for uninferable value and fix it, but I'll let you decide :)
Ah, I think I follow you now. I've got versions of the original cases, plus two new cases. |
🤖 Effect of this PR on checked open source code: 🤖 Effect on django:
Effect on pandas:
Effect on psycopg:
Effect on pytest:
Effect on sentry:
This comment was generated for commit d97504e |
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.
👌
Thanks for the reviews! |
Goodness, that was quick! My thanks to all who were involved, for this and everything else you do! |
This is going to be released in pylint 2.15.5 :) |
…values inferred (pylint-dev#7627) Add a keyword-only `compare_constants` argument to `safe_infer`.
Type of Changes
Description
If multiple values are possible for a constant, we should silence the refactoring messages that claim to offer simplifications, since the inferred value is not reliable.
Adds a keyword-only
compare_constants
argument tosafe_infer
to accomplish this in a reusable way.Closes #7626
I'm wondering if @areveny you might wish to have a look, as I saw #5227? No pressure.