search

OSGeo / grass

GRASS GIS - free and open-source geospatial processing engine
https://grass.osgeo.org
Other
828 stars 301 forks source link

style: Fixes if-stmt-min-max (PLR1730) #3950

Closed echoix closed 3 months ago

echoix commented 3 months ago

Concerns Pylint rules "consider-using-min-builtin / R1730" and "consider-using-max-builtin / R1731"

Using ruff check --output-format=concise --select PLR1730 --preview --fix.

Part of the effort to introduce Pylint 3.x for https://github.com/OSGeo/grass/issues/3921

Uses the fixes provided for ruff rule if-stmt-min-max (PLR1730) to fix part of Pylint's "consider-using-min-builtin / R1730" and "consider-using-max-builtin / R1731".

It also happens to reduce the number of branches and number of lines, where multiple places still exceeds some (already raised) limits, so its a good thing.

echoix commented 3 months ago

If it is wanted, I could. But the goal now is to have less than the couple thousand Pylint violations only for python/grass and gui/wxpython.

I made sure to leave the command used in each commit/PR. So it's really a matter of milliseconds to fix. What is longer is my manual review that I do, file by file, before committing. The PRs I submitted one after the other were the accumulation of this morning and half the afternoon. It's all from one branch, then I cherry-pick individual changed to new branches created from main, hoping that the changes were completely independent to not have any conflicts. Then I wrote the PRs.

In between the changes, I review what changed in the Pylint run, and find a new issue that ruff can easily fix.

ninsbl commented 3 months ago

Great!

The addons repo would benefit from such an effort for sure, and IMHO code style in the core and addons repo should be as similar as possible...

But of course I understand that it is a matter of priorities...

echoix commented 3 months ago

I did a little bit the other weekend on the addons-repo, as I was blocked without any reviews here, and their pre-commit config is in a better shape than here. There is still https://github.com/OSGeo/grass-addons/pull/1134 that is waiting for discussions.

If you were talking about this specific fix, you can apply ruff check --output-format=concise --select PLR1730 --preview --fix right away on that repo.