OSGeo / grass

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

[Feat] Recreate Pylint configuration from scratch #3921

Open echoix opened 3 months ago

echoix commented 3 months ago

Is your feature request related to a problem? Please describe. Pylint configuration options changed so much since the last time the pylint version changed that upgrading to a new version is almost impossible, and not all rules have equivalents, some should be added and others removed, etc.

Describe the solution you'd like To sum up, it is easier to recreate the config file from the current recommended defaults, and also make it stored inside the pyproject.toml file.

Some exclusions from the current file might need to be kept, and probably some (or many) fixes will need to be fixed in subsequent PRs. These problems should be minimally excluded in the config file, but indicate clearly which ones are technical debt to be fixed (so one can go back and remove the exclusions one by one).

echoix commented 3 months ago

I recreated the pylint config. I also added back some deviations from the defaults that were present in the config files. There are still multiple new issues to fix. I have some single-fix PRs to file, that greatly reduce the number of warnings, but it may change multiple (many) files.

echoix commented 3 months ago

I want some opinions on what should we do with a number of cyclic imports that pylint 3.x reports: image

We will need to address them at some point, but it might need a better big picture vision of the code. Who could have an idea for this?

Details (as text)

``` ************* Module wxpython gui/wxpython/__init__.py:1:0: F0010: error while code parsing: Unable to load file gui/wxpython/__init__.py: [Errno 2] No such file or directory: 'gui/wxpython/__init__.py' (parse-error) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.core -> grass.grassdb.manage) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.core -> grass.script.setup) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.script -> grass.script.core -> grass.script.setup) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.raster -> grass.script.core -> grass.grassdb.manage) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.setup) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.setup -> grass.grassdb.manage) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.script -> grass.script.setup) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.raster3d -> grass.script.core -> grass.grassdb.manage) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.vector.abstract -> grass.pygrass.vector.find) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.utils -> grass.pygrass.vector.geometry) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.utils -> grass.pygrass.vector -> grass.pygrass.vector.abstract -> grass.pygrass.vector.find -> grass.pygrass.vector.geometry) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.gis -> grass.pygrass.utils) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.gis -> grass.pygrass.utils -> grass.pygrass.vector -> grass.pygrass.vector.abstract -> grass.pygrass.vector.table) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.vector -> grass.script.core -> grass.grassdb.manage) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.gis -> grass.pygrass.utils -> grass.pygrass.vector) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.utils -> grass.pygrass.vector -> grass.pygrass.vector.abstract -> grass.pygrass.vector.find) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.pygrass.gis.region -> grass.pygrass.vector -> grass.pygrass.vector.abstract -> grass.pygrass.vector.find -> grass.pygrass.vector.geometry -> grass.pygrass.utils) (cyclic-import) gui/wxpython/__init__.py:1:0: R0401: Cyclic import (grass.grassdb.checks -> grass.script -> grass.script.db -> grass.script.vector -> grass.script.core -> grass.grassdb.manage) (cyclic-import) ```

echoix commented 3 months ago

There is also a significant number of mutable values used as a default argument. That is quite dangerous, and it was dangerous in the Python 2 time also. For example, having a list as a default value. Since default values are only evaluated once when the function is defined, then this mutable object is used across all calls to that function. It is very not good. There are multiple occurrences of this in the temporal section.

A equivalent rule in ruff is mutable-argument-default (B006)

Usually, the pattern to fix this, that I used more than 5 years ago, was to set the default value as None, and if the value is None, then apply the default value (the mutable value). However, I must know the context to make sure that the functions aren't usually called with None such as None already has a special meaning.

Not exhaustive: image image