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

style: Fixes unnecessary-collection-call (C408) for empty collections #3945

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

echoix
Copy link
Member

@echoix echoix commented Jun 30, 2024

Only applies fixes to empty collections by ruff check --select "C408" --unsafe-fixes --fix --config "lint.flake8-comprehensions.allow-dict-calls-with-keyword-arguments = true" in order to limit the review scope.

Part of preparing the repo for Pylint 3.x for #3921

Uses the fixes provided for ruff rule unnecessary-collection-call (C408) to fix part of Pylint's use-dict-literal / R1735 rule. I say "Part of", as for this PR I limited it to only the empty initializers.

Using literals instead of function calls is faster, as it doesn't require to load from global scope (to make sure "list" or "dict" wasn't changed to something else). Also, for the same reason it is better to import the functions to put them in scope (from xyz import thatfunc") instead of just using "import xyz" and then calling "xyz.thatfunc()" (it is slower to go fetch in xyz in global scope and then fetching thatfunc in that scope than importing it once, especially in loops).

Here, it says 3x faster (using [] instead of list() saves 1 opcode, and the [] avoids the LOAD_NAME opcode. https://ealexbarros.medium.com/why-is-faster-than-list-in-python-5bef48b530fc

Here, where it is analyzed for Python 3.12 in 2024, it goes in the same direction https://madebyme.today/blog/python-dict-vs-curly-brackets/, plus explains that {} uses a pre-allocated dict and adds the values if needed.

echoix added 2 commits June 30, 2024 02:10
Only applies fixes to empty collections by  `ruff check --select "C408" --unsafe-fixes --fix --config "lint.flake8-comprehensions.allow-dict-calls-with-keyword-arguments = true"` in order to limit the review scope.
@github-actions github-actions bot added GUI wxGUI related raster Related to raster data processing Python Related code is in Python libraries module general tests Related to Test Suite labels Jun 30, 2024
@echoix echoix added this to the 8.5.0 milestone Jun 30, 2024
ninsbl
ninsbl previously approved these changes Jul 1, 2024
Copy link
Member

@ninsbl ninsbl left a comment

Choose a reason for hiding this comment

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

Please see my one comment...

gui/wxpython/core/settings.py Outdated Show resolved Hide resolved
gui/wxpython/core/settings.py Outdated Show resolved Hide resolved
gui/wxpython/core/settings.py Outdated Show resolved Hide resolved
Co-authored-by: Stefan Blumentrath <stefan.blumentrath@gmx.de>
@echoix echoix enabled auto-merge (squash) July 1, 2024 02:27
@echoix echoix merged commit e99f979 into OSGeo:main Jul 1, 2024
27 checks passed
@echoix echoix deleted the fix-empty-unnecessary-collection-call branch July 1, 2024 06:11
cyliang368 pushed a commit to cyliang368/grass that referenced this pull request Jul 1, 2024
…OSGeo#3945)

* style: Fixes unnecessary-collection-call (C408) for empty collections

Only applies fixes to empty collections by  `ruff check --select "C408" --unsafe-fixes --fix --config "lint.flake8-comprehensions.allow-dict-calls-with-keyword-arguments = true"` in order to limit the review scope.

* style: apply black

* Apply suggestions from code review

---------

Co-authored-by: Stefan Blumentrath <stefan.blumentrath@gmx.de>
@a0x8o a0x8o mentioned this pull request Jul 1, 2024
a0x8o pushed a commit to a0x8o/grass that referenced this pull request Jul 2, 2024
…OSGeo#3945)

* style: Fixes unnecessary-collection-call (C408) for empty collections

Only applies fixes to empty collections by  `ruff check --select "C408" --unsafe-fixes --fix --config "lint.flake8-comprehensions.allow-dict-calls-with-keyword-arguments = true"` in order to limit the review scope.

* style: apply black

* Apply suggestions from code review

---------

Co-authored-by: Stefan Blumentrath <stefan.blumentrath@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general GUI wxGUI related libraries module Python Related code is in Python raster Related to raster data processing tests Related to Test Suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants