[POC] linting with ruff

[adebardo]

Context

The goal of this ticket is to implement linting on the code. We have noticed that certain rules applied to demcompare are not applied in the test files of xDEM.

Examples of errors present in the test files:

• tests/test_volume.py:252:11: W0125: Using a conditional statement with a constant value (using-constant-test)
• tests/test_spatialstats.py:563:12: W0612: Unused variable 'ny' (unused-variable)
• tests/test_spatialstats.py:509:0: C0115: Missing class docstring (missing-class-docstring)
• tests/test_spatialstats.py:547:4: R0914: Too many local variables (27/15) (too-many-locals)
• tests/test_vcrs.py:22:8: C0206: Consider iterating with .items() (consider-using-dict-items)

We cannot guarantee that the same code quality is applied to all the code.

Ruff

We propose using the Ruff linter to reduce the number of tools used in xdem.

• [ ] Add the .ruff.toml file

# Exclude a variety of commonly ignored directories.  
exclude = [  
    ".bzr",  
    ".direnv",  
    ".eggs",  
    ".git",  
    ".git-rewrite",  
    ".hg",  
    ".ipynb_checkpoints",  
    ".mypy_cache",  
    ".nox",  
    ".pants.d",  
    ".pyenv",  
    ".pytest_cache",  
    ".pytype",  
    ".ruff_cache",  
    ".svn",  
    ".tox",  
    ".venv",  
    ".vscode",  
    "__pypackages__",  
    "_build",  
    "buck-out",  
    "build",  
    "dist",  
    "node_modules",  
    "site-packages",  
    "venv",  
]  

# Same as Black.  
line-length = 120  
indent-width = 4  

# Assume Python 3.8  
target-version = "py38"  

[lint]  
# Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`)  codes by default.  
# Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or  
# McCabe complexity (`C901`) by default.  
select = ["ALL"]  
ignore = []  

# Allow fix for all enabled rules (when `--fix`) is provided.  
fixable = ["ALL"]  
unfixable = []  

# Allow unused variables when underscore-prefixed.  
dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$"  

[format]  
# Like Black, use double quotes for strings.  
quote-style = "double"  

# Like Black, indent with spaces, rather than tabs.  
indent-style = "space"  

# Like Black, respect magic trailing commas.  
skip-magic-trailing-comma = false  

# Like Black, automatically detect the appropriate line ending.  
line-ending = "auto"  

# Enable auto-formatting of code examples in docstrings. Markdown,  
# reStructuredText code/literal blocks and doctests are all supported.  
#  
# This is currently disabled by default, but it is planned for this  
# to be opt-out in the future.  
docstring-code-format = false  

# Set the line length limit used when formatting code snippets in  
# docstrings.  
#  
# This only has an effect when the `docstring-code-format` setting is  
# enabled.  
docstring-code-line-length = "dynamic"

• [ ] add ruff in makefile

  lint: ruff check $(SRC_DIRS)

• [ ] add ruf in CI

  name: Run 
  Ruff run: | 
      ruff check .

• [ ] add ruff in setup.cfg

Correction

• [ ] Ruff return lots of problem that needed to be fixed

Estimate

3d

[adebardo]

@adehecq , @duboise-cnes tell me if you agreed

[duboise-cnes]

on the general linting, i agree to put something at the beginning to benefit from the quality wrappers for all code to be done afterwards. I think it is a must have with this too permissive python language. I would nevertheless prefer go to :

• quicker ruff directly for replacement of pylint (not all but i think sufficient and growing), black, flake8, isort. only one tool easier
• and mypy very important also (type checking is really important). Maybe another ticket.

See https://docs.astral.sh/ruff/faq/ and ruff is now really mature and faster. So it would be a better choice on linting i would advise

"Explain why pylint is in the "test" dependencies in the setup file and see if it's the most appropriate place for it" --> you mean use a "dev" target ? more explicit but if xdem is used with test for dev parts, go for it. No casus belli ;)

max line to 120 is a lot for my old me used of little screen but it is the dev standard so ok for me if the code is already there ;)

max-module-lines is also important for refactoring goal. 700 or 1000 max could be another issue in parallel of code structure possible refactoring

[adebardo]

Hello, I am glad you suggested using ruff. I will revise the ticket with this solution. And an think about a refactoring for smaller files

[adebardo]

Ruff seems bad as it job :

with ruff :

with pylint:

[duboise-cnes]

Ok see internally if it is misconfiguration of ruff and if not enough mature (it's weird nonetheless) I think we keep isort, black, flake8, pylint, mypy as a baseline.

[adehecq]

I didn't know riff, but I think if we could reduce the number of linters to one that manages all, that would certainly make things easier. It's seems normal that ruff does not complain in some parts while pylint does. As stated in the FAQ, ruff and pylint do not completely overlap on rules. Maybe this can be tuned a bit with configuration, but otherwise we just have to accept that no tool will perfectly catch all issues.

As for the line length, 79 is really too short when there are 2, 3, 4 indentation levels... 120 is a bit more comfortable. For the maximum number of lines, let's try to not be too strict for the moment, as we have rather long files in geoutils/xdem at the moment 😅

[rhugonnet]

Agree on all. For ruff, I think @erikmannerfelt mentioned it about a year ago. At that time the main improvement seemed to be speed for very large projects that had to wait several minutes to perform pre-commit. This is not a problem on this repo yet, but it would be more durable to move to ruff anyways for sure, so all for it as well :slightly_smiling_face:

[adebardo]

I finally found a great config file for Ruff, and it seems suitable for our project. I didn’t ignore any rules, so we can discuss them during development if needed.

P.S.: We can also choose the line length.