warehouse icon indicating copy to clipboard operation
warehouse copied to clipboard

Investigate replacing `pyupgrade`, `black`, `isort` and `flake8` with `ruff`

Open facutuesca opened this issue 11 months ago • 4 comments

Since I couldn't find any discussion about ruff in existing issues, I'm opening this one to see if there's interest and to document what I've found so far.

Currently, Python linting and formatting (via make lint and make reformat) is done via black, isort, pyupgrade and flake8.

ruff can be used as an (almost) drop-in replacement for most of these. The only thing not supported is custom flake8 plugins, like the one defined here.

I have a branch where I tested the migration by:

  • Replacing pyupgrade with ruff's pyupgrade rules
  • Replacing isort with ruff''s isort rules
  • Replacing black with ruff format
  • Replacing flake8's default rules with ruff check
  • Replacing flake8-pytest-style default rules with ruff's flake8-pytest-style rules
  • Still running flake8, but only selecting the custom WH plugin mentioned above.

After migrating, running with a warm cache I get around a 10 second improvement in both make commands, and make reformat is now almost instant:

Before (black+pyupgrade+isort+flake8):

make reformat  0.08s user 0.04s system 1% cpu 10.757 total

make lint  0.15s user 0.07s system 0% cpu 33.592 total

---------------

After (ruff+flake8):

make reformat  0.07s user 0.04s system 14% cpu 0.754 total

make lint  0.14s user 0.07s system 0% cpu 23.736 total

cc @miketheman @woodruffw

facutuesca avatar Feb 06 '25 19:02 facutuesca

Thanks for taking a stab at this! I recall @ewdurbin tried this out once, can't find the PR right now.

Thanks for confirming that you continued to check with the custom WH flake8 rules - I kinda wish ruff would bridge the Rust-Python divide and be able to directly import and use flake8 checkers, but that's not the case right now.

In my mind, migration should be due to a compelling reason, and ruff uses caching heavily, so we'd need to be cognizant of how those caches are stored, invalidated, and managed to support both in-docker development, as well as GitHub Actions so we can continue to benefit from speedups.

The slowest parts of the current linting are curlylint and mypy - neither of which are ruff-supported yet - did your tests exclude these from timings?

miketheman avatar Feb 10 '25 20:02 miketheman

The slowest parts of the current linting are curlylint and mypy - neither of which are ruff-supported yet - did your tests exclude these from timings?

No, the test numbers include curlylint and mypy running in make lint

facutuesca avatar Feb 10 '25 22:02 facutuesca

No, the test numbers include curlylint and mypy running in make lint

Thanks for confirming.

Honestly, I don't find the speedup that compelling to justify the changes.

If any other Admin wants it, that's fine too, but I'm a -1 for now on the tooling changes, and would prefer focus on other improvements.

miketheman avatar Feb 24 '25 21:02 miketheman

Sounds good. I think these numbers are great for local runs (IMO a 90% improvement on warm reformat runs is amazing), but it's true that CI won't benefit as directly.

I think we can icebox this on our side and revisit in the future. Thanks for looking into this @facutuesca!

woodruffw avatar Feb 24 '25 21:02 woodruffw