DEV: Drop black and flake8 and use ruff instead

[herebebeasties]

Fixes #325

Note that this configuration disables a couple of things to avoid making a ton of noise in this PR:

• E501 is ignored (line length):
  • I've set line-length=88 but there are a ton of places where auto-wrapping isn't possible due to huge trailing comments
• "I" lint rule, which is all the isort stuff
  • exchange_calendars imports aren't currently sorted using isort so this would make this PR enormous and annoying to review
  • If we'd like to turn that on (which I think we should) then I will make a noisier PR after this one has landed which does only that

Also, I highly recommend the official ruff plug-in for VS Code. It's great.

This PR is split into three commits to make it easier to review:

• Tooling
• Fixes
• Formatting

Note

Needs #369 and #370 to be landed first before the pre-commit checks will then pass.

[maread99]

This is great! Thank you very much! Indeed, thank you for all the recent PRs and comments to Issues. I'm half expecting another PR to suddenly pop up from you with the core implementation in Rust! 😀

This one will be for @gerrymanoim to review. FWIW, I'm not sure I'm fully comfortable changing the formatter from black given that it's the de facto standard python formatter. Maybe my unease is unwarranted. I see the two are highly compatible (at least at the moment).

Cheers

[maread99]

If we were to move to the ruff formatter, just noticed that the code style badge at the top of the readme would probably need changing.

[herebebeasties]

That is obviously for you as project maintainers to decide, but fwiw I've moved much larger codebases over to ruff and found it to be 100% Flake8/isort/Black-compatible, just a lot faster. As in, format-on-save-in-the-IDE-and-don't-even-think-about-it kind of fast.

There are a couple of very minor corner case differences with the isort implementation where isort does weird things but that's about it. It's very well-established now, is fewer dependencies, and has been a very pleasant experience.

I'll patch the README and merge up-to-date to make the checks now go green. Thanks.

[maread99]

Absolutely sterling. Thanks again, and for offering your experience with ruff.

I think I've just got an emotional attachment to black and struggle to look past it. I certainly wouldn't mind if Gerry decides that the ruff formatter is the way to go.

[herebebeasties]

You're very welcome. https://github.com/astral-sh/ruff/blob/main/docs/formatter.md#philosophy might help you get over that:

The initial goal of the Ruff formatter is not to innovate on code style, but rather, to innovate on performance, and provide a unified toolchain across Ruff's linter, formatter, and any and all future tools.

Specifically, the formatter is intended to emit near-identical output when run over existing Black-formatted code. When run over extensive Black-formatted projects like Django and Zulip, > 99.9% of lines are formatted identically. (See: Black compatibility.)

You might also want to look at their testimonials.

But if Gerry decides he wants to keep using Black then I can always update this PR to just swap out the flake8 bits. You'll be missing out, though. :-)

[gerrymanoim]

Sorry why do we need --exit-non-zero-on-fix?

[herebebeasties]

Otherwise it'll exit status code zero, which means pre-commit will merrily let you land the changes with outstanding linting issues. This way the commit fails, you review & add the auto-fixes that ruff has made, and you try the commit again.

[gerrymanoim]

merrily let you land the changes with outstanding linting issues.

But in this case the changes are fixed, no?

[herebebeasties]

It turns out that pre-commit fails if a hook changes any of the files that the user has staged (without then doing a git add) so I agree that this looks superfluous. I'll remove it.

[herebebeasties]

Hmmm. I say that, but...

We're running this in two ways:

• In the local developer's git repo, only if they have pre-commit set up.
• In GitHub Actions, which enforces linting regardless of a dev's local setup.

So we do need this to exit non-zero really, for GitHub Actions to fail the build if there are linting fixes. We don't want them to silently get applied within the CI env but never committed back and for it to pretend it's all fine.

[gerrymanoim]

I think the easiest thing to do here would be to:

1. Keep the precommit with just args: [--fix]
2. Instead of running the precommit GHA, run ruff directly via any of https://docs.astral.sh/ruff/integrations/#github-actions

[gerrymanoim]

Sorry for the delay all - was on vacation.

I'm very pro ruff (and the whole astral ecosystem) so very welcome change. A few small questions, but overall lgtm.

I'd love to get to the default ruff config + extend-select = ["I"], but we can do that as a follow up. I think doing a separate large reformat would be best, but open to suggestions.

[herebebeasties]

While I agree you could do that, surely you want the other pre-commit checks to run on GitHub anyway?

We run with this exact set-up and find it works well, but I'm obviously happy for you to change this however you see fit. I have no time to look at this further in the next few days though, I'm afraid.

[gerrymanoim]

While I agree you could do that, surely you want the other pre-commit checks to run on GitHub anyway?

We run with this exact set-up and find it works well, but I'm obviously happy for you to change this however you see fit. I have no time to look at this further in the next few days though, I'm afraid.

My main desire is to make the dev process as smooth as possible, and I think having ruff auto fix everything without you having to re-commit the files helps there. If that means we don't run the trailing whitespace checker/yaml checker, I'm okay with that tradeoff.