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

Replace all python linters (flake8, pylama,...) with ruff #140

Closed
wants to merge 8 commits into from
Closed

Replace all python linters (flake8, pylama,...) with ruff #140

wants to merge 8 commits into from

Conversation

ThibFrgsGmz
Copy link
Contributor

@ThibFrgsGmz ThibFrgsGmz commented May 13, 2023

Originating Project/Creator
Affected Component Static Analyzer
Affected Architectures(s) Infrastructure
Related Issue(s) None
Has Unit Tests (y/n) n
Builds Without Errors (y/n) see CI
Unit Tests Pass (y/n) see CI
Documentation Included (y/n) n

Change Description

Ruff, by Astral is a very fast linter implemented in Rust that can replace pylama, flake8, ....

Ruff also implements the rules of other popular Python linters such as pylint, isort and pyupgrade. There are even plans to implement feature parity with black in the future. Ultimately, it can become our one-stop shop for all things Python linting and formatting.

  • Added a first configuration file to get global codebase defaults for all Ruff checks.
  • Add Ruff output to the GitHub security interface (SARIF output format is in development).
  • Decide whether to enable all rules and ignore some or the other way around (enable some and keep the ignore list empty)
  • Decide which rule we allow to be corrected by the tool
  • Indicate the category/rules to be respected
  • Open tickets asking to correct violated categories (not in the scope of this PR)
  • Choose a good complexity level for McCabe rules (trigger when complexity is above a specific level)
  • Choose the docstring convention to use (https://beta.ruff.rs/docs/faq/#does-ruff-support-numpy-or-google-style-docstrings)
  • Remove pylama/flake8/... configuration files

Rationale

This stack should replace all our Python lint tools with ruff, except for:

  • black for formatting,
  • type-checker (Mypy, Pyright, or Pyre) see here.

It provides:

  • better linting with faster feedback (blazingly fast)
  • an auto-fix feature

Testing/Review Recommendations

To avoid noise in others' patches, violated rules are currently all ignored.

Configuration

Ruff is configured in the root file pyproject.toml. In addition, ruff retrieves any pyproject.toml file.
In addition, ruff retrieves any pyproject.toml or ruff.toml files in the subdirectories. The settings in these files will only apply to the whole system.
The settings in these files will only apply to the files contained in these subdirectories.

Additional information

Future Work

  • Add a type checker (Mypy, Pyright, or Pyre) as a complement.
  • Continuously follow the versions of the tool to benefit from the latest checks added.
  • Add FPrime as an open-source project using this tool by adding it to the README of the Ruff core.
  • Add the same Ruff configuration to FPrime-gds

[tool.ruff]
# Configuration: https://beta.ruff.rs/docs/configuration/
# Rules: https://beta.ruff.rs/docs/rules/
target-version = "py38" # Pin Ruff to Python 3.8
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomas-bc I put Py3.8 minimum as the python 3.7 will be deprecated next months (no security support). So according to FPrime rules, Python 3.8 will be the minima version supported.

@thomas-bc
Copy link
Contributor

I'll have to take some time to sit down and look at all this, but this looks really promising. Thank you @ThibFrgsGmz ! I'll get to this as soon as I can.

@ThibFrgsGmz
Copy link
Contributor Author

For the time being, it's best to wait for the SARIF output format to be supported. That's for the best, as other rules will be added in the meantime.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants