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

Minor optimizations by using comprehensions #2771

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Mar 25, 2024

% pipx install ruff
% ruff check --select=C4,PERF --statistics

30	C408   	[*] Unnecessary `dict` call (rewrite as a literal)
10	C406   	[*] Unnecessary `tuple` literal (rewrite as a `dict` literal)
10	C417   	[*] Unnecessary `map` usage (rewrite using a generator expression)
 9	PERF401	[ ] Use a list comprehension to create a transformed list
 2	C419   	[*] Unnecessary list comprehension
 2	PERF203	[ ] `try`-`except` within a loop incurs performance overhead
 2	PERF402	[ ] Use `list` or `list.copy` to create a copy of a list

% ruff check --select=C4,PERF --fix --unsafe-fixes

Found 65 errors (52 fixed, 13 remaining).

% ruff rule C408

unnecessary-collection-call (C408)

Derived from the flake8-comprehensions linter.

Fix is always available.

What it does

Checks for unnecessary dict, list or tuple calls that can be
rewritten as empty literals.

Why is this bad?

It's unnecessary to call, e.g., dict() as opposed to using an empty
literal ({}). The former is slower because the name dict must be
looked up in the global scope in case it has been rebound.

Examples

dict()
dict(a=1, b=2)
list()
tuple()

Use instead:

{}
{"a": 1, "b": 2}
[]
()

Fix safety

This rule's fix is marked as unsafe, as it may occasionally drop comments
when rewriting the call. In most cases, though, comments will be preserved.

Options

  • lint.flake8-comprehensions.allow-dict-calls-with-keyword-arguments

@cclauss cclauss requested a review from a team as a code owner March 25, 2024 18:35
buildconfig/config.py Outdated Show resolved Hide resolved
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lint job seems to have failed with other errors on this PR, so a commit fixing all those will also be needed

@cclauss
Copy link
Contributor Author

cclauss commented Mar 26, 2024

Are you talking about #2773 or something else?

@ankith26
Copy link
Member

I'm talking about this fail: https://github.com/pygame-community/pygame-ce/actions/runs/8425378679/job/23088593026

@cclauss cclauss requested a review from ankith26 March 26, 2024 14:20
@ankith26 ankith26 added the Code quality/robustness Code quality and resilience to changes label Mar 27, 2024
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR 🥳

@Starbuck5
Copy link
Member

This PR now has merge conflicts.

@cclauss cclauss force-pushed the use-comprehensions branch from 764833b to 9d48251 Compare March 29, 2024 08:00
@cclauss
Copy link
Contributor Author

cclauss commented Mar 29, 2024

Git conflicts fixed.

@ankith26 ankith26 added this to the 2.5.0 milestone Apr 4, 2024
Copy link
Member

@zoldalma999 zoldalma999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@zoldalma999 zoldalma999 merged commit 3e661e1 into pygame-community:main Apr 26, 2024
29 checks passed
@cclauss cclauss deleted the use-comprehensions branch April 26, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality/robustness Code quality and resilience to changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants