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

chore: fix pylint message use-set-for-membership #882

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

jenstroeger
Copy link
Contributor

@jenstroeger jenstroeger commented Sep 30, 2024

This change is part of issue #876 and addresses the use-set-for-membership check messages.

I’ll add the appropriate configuration to pyproject.toml in a separate PR later.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 30, 2024
@tromai
Copy link
Member

tromai commented Oct 2, 2024

I wonder if this check flags an error on cases where you are performing a membership test in cases like this?

a: dict[str, str] = get_some_dictionary(...)
for key in a:
    ...
import glob

for filename in glob.iglob("*.txt"):
    print(filename)
a: list[str] = get_some_list(...)
for ele in a:
    print(ele)

Or does it only flag cases where we perform membership test on "statically-defined" lists

fruit in ["foo", "boo"]

@jenstroeger
Copy link
Contributor Author

jenstroeger commented Oct 2, 2024

I wonder if […]

Why don’t you just try? 😉

@tromai, however, the code in your examples above are not membership tests: a for statement iterates over a collection, it does not check whether a given item exists in the collection.

[…] if this check flags an error on cases […]

Note also that these are not “errors” per se; the optional pylint checks are mostly recommendations for and improvements of code quality and performance.

See PR #890 on how to add the plugin; or, if you feel adventurous, you can read the checker’s code here

@jenstroeger jenstroeger force-pushed the lint/use-set-for-membership branch from 04e4b75 to e0fd295 Compare October 2, 2024 03:42
@jenstroeger jenstroeger force-pushed the lint/use-set-for-membership branch from e0fd295 to 10140a7 Compare October 2, 2024 05:08
@behnazh-w behnazh-w merged commit b23fab1 into oracle:staging Oct 2, 2024
9 checks passed
@tromai
Copy link
Member

tromai commented Oct 2, 2024

however, the code in your examples above are not membership tests: a for statement iterates over a collection, it does not check whether a given item exists in the collection.

Ah, thanks for pointing it out. I made some mistakes when coming up with the examples, please see the revised version here:

a: dict[str, str] = get_some_dictionary(...)
return if "key" in a.keys()
# OR: return if "key" in a
import glob

return "boo.txt" in glob.iglob("*.txt")
a: list[str] = get_some_list(...)
return "boo" in a
b = [1,2]
c: list[int] = get_int_list(...)
d = [4,3]
a = [b, c, d]

c in a

Note also that these are not “errors” per se; the optional pylint checks are mostly recommendations for and improvements of code quality and performance.

I should have been a bit more explicit in my wordings. I meant whether pylint would exit with an error/non-zero status code for those cases, but not about the content of the checks.

art1f1c3R pushed a commit that referenced this pull request Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants