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: Fix unused-import (F401) #4521

Merged
merged 30 commits into from
Oct 23, 2024
Merged

Conversation

echoix
Copy link
Member

@echoix echoix commented Oct 14, 2024

Ruff rule: https://docs.astral.sh/ruff/rules/unused-import/
Also adjusted the flake8 config file.

These commits started from an exploratory branch last week where I was profiling temporal tests. Removing an import of SimpleModule when not needed ended up with a measurable improvement in temporal/t.rast.accumulate/testsuite/test_accumulation.py, when removing all cases except one, and benchmarking with hyperfine (10 runs) each case, on Windows and Linux. I don't have the correct output for these isolated changes, as it was combined with more changes, but if combined with using t.remove with 3 layers in the input argument instead of calling t.remove 3 times, the fastest case was 1.18x faster than base, and 1.15x faster than base (I don't remember what conditions it was, but there was one less change). But the base was already modified by a change in PLY, sot it doesn't match exactly what was on main here.

The other change to t.remove will be in another PR.

Ignored inline where the import is justified and isn't to fix later, and ignored in a per-file exclusion where the imports seemed to be for commented-out test case lines
@echoix echoix added this to the 8.5.0 milestone Oct 14, 2024
@github-actions github-actions bot added vector Related to vector data processing raster Related to raster data processing labels Oct 14, 2024
@github-actions github-actions bot added temporal Related to temporal data processing Python Related code is in Python database Related to database management libraries module docs tests Related to Test Suite notebook labels Oct 14, 2024
@echoix echoix requested review from petrasovaa and wenzeslaus and removed request for wenzeslaus October 16, 2024 21:34
@petrasovaa
Copy link
Contributor

How does the __all__ thing work exactly?

@echoix
Copy link
Member Author

echoix commented Oct 17, 2024

How does the __all__ thing work exactly?

It explicitly lists what is available (exposed) when using star imports with the from module import * syntax.

https://docs.python.org/3/tutorial/modules.html#importing-from-a-package

https://www.geeksforgeeks.org/python-__all__/

https://medium.com/@akshatgadodia/demystifying-all-in-python-a-closer-look-at-module-exports-f4d818a12bb6

@echoix
Copy link
Member Author

echoix commented Oct 21, 2024

@arohanajit Just to let you know that this PR will fix all (or most known, detected through ruff's implementation) of the F401, so you don't spend time in it. It's just still waiting to be merged.

.flake8 Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
doc/notebooks/parallelization_tutorial.ipynb Outdated Show resolved Hide resolved
echoix and others added 2 commits October 23, 2024 11:36
@echoix echoix enabled auto-merge (squash) October 23, 2024 17:56
@echoix echoix merged commit 533070d into OSGeo:main Oct 23, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Related to database management docs libraries module notebook Python Related code is in Python raster Related to raster data processing temporal Related to temporal data processing tests Related to Test Suite vector Related to vector data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants