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

[Adjust Rule]: SIM204, SIM205, SIM206, SIM207 on sets #116

Closed
Nathanmalnoury opened this issue Mar 25, 2022 · 2 comments
Closed

[Adjust Rule]: SIM204, SIM205, SIM206, SIM207 on sets #116

Nathanmalnoury opened this issue Mar 25, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@Nathanmalnoury
Copy link

Nathanmalnoury commented Mar 25, 2022

Hi, thanks a lot for this flake8 library, It's been helping us a lot with keeping a nice code quality 1

I think we've spotted a small issue when it comes to replacing not < with >= rules using sets.

Desired change

  • Rule(s): SIM204, SIM205, SIM206, SIM207
  • Adjustment:

Using sets, saying not (a < b) is not equivalent to a >= b:

  • not (a < b): there is no element of a that is in b
  • a >= b: all elements of b are in a

I am unsure what the adjustment should be, maybe omit these rules when comparing sets ?

Explanation

On sets, (not (a < b)) != (b >= a)

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

a = {4, 5}
b = {1, 2, 3}

# this raises: SIM204 Use 'a >= b' instead of 'not (a < b)'
assert not (a < b)   # no elements of a are in b; would pass 

# when making the suggesting change:
assert a >= b  # all elements of b are in a -> AssertionError
@Nathanmalnoury Nathanmalnoury added the enhancement New feature or request label Mar 25, 2022
@Nathanmalnoury Nathanmalnoury changed the title [Adjust Rule] [Adjust Rule]: SIM204, SIM205, SIM206, SIM207 on sets Mar 25, 2022
@MartinThoma
Copy link
Owner

Thank you very much! I wasn't aware that sets could be compared.

Understanding the issue

If I get it right, then a < b is equivalent to a.issubset(b)?

And b < a is equivalent to b.issuperset(a)?

Fixing the issue

If that is the case, then those rule suddenly becomes WAY more complicated. The way flake8-simplify works is on the abstract syntax tree. At this level, I don't have any type information. I might need to disable those 4 rules to prevent false-positives 🥲

@Nathanmalnoury
Copy link
Author

I guess you meant a > b in your second example:

  • a < b is a.issubset(b) and a != b
  • a > b is a.issuperset(b) and a!= b

(Here is the link to python's doc about them: https://docs.python.org/3/library/stdtypes.html#frozenset.issubset)

I think you might have to disable those 4 rules indeed 😬 Sorry about that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants