Skip to content

Enforce ruff/flake8-comprehensions rules (C4) #4785

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Enforce ruff/flake8-comprehensions rules (C4) #4785

wants to merge 10 commits into from

Conversation

DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Jan 1, 2025 •
edited
Loading

Summary of changes

Pull Request Checklist

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 1, 2025

⚠️ The sha of the head commit of this PR conflicts with #4386. Mergify cannot evaluate rules on this PR. ⚠️

1 similar comment
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 5, 2025

⚠️ The sha of the head commit of this PR conflicts with #4386. Mergify cannot evaluate rules on this PR. ⚠️

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review March 11, 2025 13:52
@Avasam
Copy link
Contributor

Avasam commented Mar 11, 2025 •
edited
Loading

I think you'll want to enable allow-dict-calls-with-keyword-arguments
I'd have to find it back, but I believe that's a stylistic decision @jaraco has previously weighed in.

@DimitriPapadopoulos DimitriPapadopoulos force-pushed the C4 branch 3 times, most recently from bb995d6 to 5e77363 Compare March 11, 2025 20:26
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Mar 12, 2025 •
edited
Loading

After adding allow-dict-calls-with-keyword-arguments, most occurrences of C408 have disappeared. A few instances of dict() → {} have been left behind. Should I discard them as well and disable C408?

@Avasam
Copy link
Contributor

Avasam commented Mar 12, 2025

I personally think those make sense, it's mainly changing generators of tuples into dict comprehensions.

@DimitriPapadopoulos DimitriPapadopoulos force-pushed the C4 branch 2 times, most recently from ef18ee2 to e525809 Compare April 14, 2025 08:56
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the C4 branch 2 times, most recently from 412819f to 36fa684 Compare May 23, 2025 08:13
@DimitriPapadopoulos
Copy link
Contributor Author

Rebased.

@DimitriPapadopoulos
Copy link
Contributor Author

@Avasam
Copy link
Contributor

Avasam commented May 23, 2025

I'm not sure your rebase is correct. I see refurb related changes and the change dict(foo=bar) --> {"foo": bar} which was previously mentioned as unwanted.

PyPy tests have also been flaky: #4940

@DimitriPapadopoulos
Copy link
Contributor Author

I had to rerun C401 and C408 because rebasing was impossible. I guess I somehow got the options wrong. Will try again.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 23, 2025 •
edited
Loading

Reapplied C408 after making sure allow-dict-calls-with-keyword-arguments is true.

@DimitriPapadopoulos
Apply ruff/ flake8-comprehensions rule C400
18b3d81 
C400 Unnecessary generator (rewrite as a `list` comprehension)
@DimitriPapadopoulos
Apply ruff/flake8-comprehensions rule C401
7a33380 
C401 Unnecessary generator (rewrite as a `set` comprehension)
@DimitriPapadopoulos
Apply ruff/flake8-comprehensions rule C402
1f71c21 
C402 Unnecessary generator (rewrite as a `dict` comprehension)
@DimitriPapadopoulos
Apply ruff/flake8-comprehensions rule C405
36a176f 
C408 Unnecessary `dict()` call (rewrite as a literal)
C405 Unnecessary `list` literal (rewrite as a `set` literal)
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the C4 branch 2 times, most recently from ff4292e to 051b475 Compare May 23, 2025 15:48
@DimitriPapadopoulos
Manual improvement for rule C405
61c117f
@DimitriPapadopoulos
Apply ruff/flake8-comprehensions rule C413
5fe2139 
C413 Unnecessary `list` call around `sorted()`
@DimitriPapadopoulos
Apply ruff/flake8-comprehensions rule C414
0b1daed 
C414 Unnecessary `list` call within `sorted()`
@DimitriPapadopoulos
Apply ruff/flake8-comprehensions rule C417
e3fb64b 
C417 Unnecessary `map` usage (rewrite using a generator expression)
@DimitriPapadopoulos
Apply ruff/flake8-comprehensions rule C420
da9cbb3 
C420 Unnecessary dict comprehension for iterable; use `dict.fromkeys` instead
@DimitriPapadopoulos
Enforce ruff/flake8-comprehensions rules (C4)
4152840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DimitriPapadopoulos @Avasam