The current way we check python code is:
- Specify a flake8 configuration matching the style guide.
- Run flake8 in a GitHub action to ensure compliance before merging is allowed.
- Run pytest-flake8 when we run the tests (generally via sconsUtils but also using a pytest configuration that adds the "--flake8" option to the pytest call).
The pytest-flake8 dependency is causing real problems because the package is moribund and it does not support anything newer than flake8 v4. This forces us to pin flake8 to an older version in the rubin-env. There is no hope that pytest-flake8 is going to be resurrected. Additionally, the pytest-flake8 calls are slow since flake8 is run separately for every file and there is no multi-processing internal to flake8. For local development this is not much of a problem because on the second call to pytest the cache is used, but it does add load to Jenkins. When pytest reports durations during builds you will see that at times a FLAKE8 test appears in the top 5 (maybe taking 3 seconds for a single file). As a concrete example, the daf_butler tests on my computer take 408s including the flake8 (no multi-processing) and 300s (no multi-processing) after that when flake8 results have been cached. This overhead is less noticeable when tests run in parallel but it is still there.
If flake8 is run standalone and allowed to use multiple cores (flake8 v4 does not allow this on macOS), something like daf_butler can complete the test in 2 or 3 seconds. There is no caching so every time it takes the same time.
There is though an alternative to flake8 called ruff that has a number of advantages:
- It is compatible with flake8 (uses the same error codes) and can be a drop-in replacement with a suitable configuration.
- It is incredibly fast and includes caching to make it even faster.
- It comes by default with additional checkers so new checks can be added without requiring a new support package to be installed into the environment. For example, enabling pydocstyle requires solely adding a "D" to the ruff configuration. Enabling "UP" will show you where you can modernize your python code.
- In some instances it can automatically fix problems (most "UP" errors and some "D" errors).
- Like flake8 it can be run by pre-commit.
The timing improvements are stark. 0.25s for daf_butler compared to 3 seconds (but using much less CPU) and 0.02s subsequently. Indeed, the entirety of lsst_distrib takes only 3 seconds the first time it runs.
This RFC proposes the following:
- Drop pytest-flake8 from all packages.
- Change sconsUtils to always run a linter if a suitable configuration is available.
- Run ruff if a ruff configuration is available, run flake8 otherwise (do not run flake8 if no .flake8 or setup.cfg configuration exists). This allows each package to define when it should switch to using ruff.
- Add ruff to rubin-env (it must be available in the main build)
- Modify the style guide to indicate that ruff MAY be used for linting (it currently says that flake8 MAY be used).