GlacioHack
[WIP] Implement linting with Ruff
Resolves #550
Description :
The Pull Request implements linting on xDEM in order to guarantee the same code quality as demcompare. The idea was to limit the number of tools and to apply the same rules as demcompare. That's why we choose to implement Ruff in Xdem, a fast python linter and code formatter gathering a lot of functionalities and replacing pylint, black, flake8 and isort.
Key changes :
-
Ruff configuration :
-
[x] ruff.toml file addded including the following rules :
- Target version of python : 3.10
- The formatter will wraps lines at a length of 120 and use 4 spaces per tabulation when a line exceed 120 characters.
- Ruff will ignore 46 lint rules (for now)
-
[x] Ruff added in makefile
-
[x] Ruff added in CI file
-
[x] Ruff added in setup.cfg file
-
-
Code correction :
Ruff returned a lot of errors related to 108 lint rules :
- [x] 62 of them has been fixed, the remaining is ignored and can be fixed later
In the tests, the protected attributes (like _need_vars) were changed to non-protected attributes (for example, need_vars). This is causing issues because the non-protected attributes may not actually exist in the class in the same form, which leads to errors. Protected attributes (with a leading underscore) are intentionally kept as internal members of the class and might not have corresponding non-protected versions.
It seems that these changes could have been made in response to warnings from Ruff or due to the use of the --fix option. However, the removal of the leading underscore breaks the logic, since the non-protected version of these attributes may not exist.
To fix this:
- The protected attributes should be restored with their leading underscores.
- We may also want to review the Ruff configuration to ensure it's not suggesting incorrect automatic changes.
To apply ruff in pre-commits and avoid conflicts with other linters, you would need to add ruff to .pre-commit-config.yaml and remove isort, pylint, flake8 and black.
Great! :star_struck: Tell me if you need more feedback on this PR :)
Question: I heard that Ruff was a "drop-in replacement" for black, flake8, etc (so producing almost exactly the same output, just much much faster). So most of the changes should arise mostly from updating black and other to more recent versions, or different rules, right? (Out of curiosity, did you try to mirror the current configuration to check that there were almost no changes?)
This PR would become much easier to merge if you disable most ruff rules and re-enable them one by one.
