-
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
[ruff
] Implement compound comparison rule (RUF029
)
#10028
Conversation
I don't think the |
This pylint rule is related (to the introduction mostly): Perhaps you'd be interested to implement it too |
Huh, I thought that was already implemented in Ruff. Thanks for the idea @sbrugman, I'll implement it tomorrow. |
Make sense for this to be a pylint rule more than a RUF rule. |
I am not aware of there being a corresponding Pylint rule on which to map, is there one? |
Yeah, in RUFF it would be |
I might be misunderstanding something, sorry. That turns things into compound comparisons (have a PR for that tomorrow). This rule does the opposite. They should be separate rules, no? |
Indeed, I understand it the same way. The current PR implements a new rule that does not exist in pylint (although it would be a good fit there) and thus should be assigned the latest available RUFF code, e.g. RUF029. The PLR1716 is the linked related rule that is included by Pylint, but not implemented in the PR. |
Ah you are right, my bad. |
I'm undecided how we should categorize this rule. To me it's a restriction rule because it disallows a subset of Python's grammar. However, I could also see that the rule fits into suspicious because the code probably doesn't do what you intended. @AlexWaygood what do you think? |
I think it's a useful rule to enforce a Python style that's more readable, but I'd certainly agree that it should be disabled by default. |
Thank you @tjkuson for working on this rule and sorry for the late feedback. We agree that the rule itself is valuable and think it should be added to Ruff eventually. But we don't feel comfortable doing it today before #1774 is merged. The rule restricts the allowed Python syntax in an opinionated way, which we aren't comfortable enabling by default but all users that use That's why we want to hold back with the rule for now but hope to get back to it after #1774 is completed. Sorry that we only decided this after you implemented the rule (without mentioning on the issue) |
@MichaReiser No worries! |
Summary
Implements
non-transitive-compound-comparison
(RUF029
) that disallows certain compound comparisons.As suggested,
Not a huge fan of the name I gave it, though. Chose
RUF029
asRUF028
has been used in a bunch of open PRs.Closes #8964.
Test Plan
cargo nextest run