Migrate to ruff

[calumy]

This PR proposes using Ruff for linting.

The flake8 and isort configs have been migrated initially, but further lint checks could be enabled in the future.

As this repo has pre-commit CI installed, I do not believe that linting needs to be included in the test workflow, so both flake8 and isort have been removed from the tox config.

Currently, ruff does not allow for multi_line_output; the discussion around this can be found here. As this only affects one import, I don't see this as too egregious of a change.

[johnraz]

I'm not fully onto the ruff bandwagon yet... Having to read / write Rust to debug Python linting tool is still kinda pushing me back...

Is there any other improvements apart from performances from switching?

I would be expecting a very low impact on perf for such a small project as this one, maybe you can provide some metrics ruff vs current pre-commit hooks?

I feel like switching to ruff could make sense if we suffered from slowlness which I doubt we are.

I am quite against making a change to something that just works and see this as an open door to problems we don't have 😉

E.g: the issue you are mentioning (astral-sh/ruff#2600) is open for a year without much activity - we currently don't have such a problem with the current linters

Sorry if I sound pedantic, just trying to be pragmatic, I'm open to more valid arguments in favor though

[giovannicimolin]

@calumy Thanks for your contribution to this repo!

@johnraz Echoed my thoughts in this PR: I don't think it's worth jumping into a new tool for the sake of using a new tool if it doesn't address any issues that this repository currently has.

This is a very small repository, with a simple purpose, so I'm in favor of keeping all tooling and dependencies to a minimum and using well known tools in the community.

Let me know your thoughts though, I'm curious to see why you picked ruff and how it can help this repository.

[calumy]

@johnraz and @giovannicimolin, thanks for your feedback.

I think we should probably consider splitting this PR into two different discussion points:

1. The removal of linting items from testing as pre-commit CI is already installed
2. The potential migration of linting from two tools (flake8 and isort) to ruff

Taking each point in turn.

Duplicate CI stages

The repo is currently configured to run linting in two places: pre-commit CI and the Python 3.11 stage of the tox config. Both locations are configured in different ways. In tox, flake8 runs on only the knox directory; however, in pre-commit ci, it runs on the whole repo. For isort in tox, it runs on an alternative subset of files, and in pre-commit ci, it runs on all files in this repo. An example of this can be seen in the commit that added flake8 that the test.py was now linted by flake8 and needed to be fixed.

If you have a lint failure, it will occur in both places. Failing the Python 3.11 branch of testing if all the tests pass seems counterintuitive. Therefore, if you are happy with it, I would propose opening an alternative PR to remove linting from the tox config.

Migration to ruff

I don't see performance as the prime driver for proposing Ruff as a tool for this repo. currently, ruff is 24x faster, but both are less than 0.25 seconds when run locally.

Other advantages, including suggestions/auto-fixes, the tool's all-in-one nature and lsp/vscode extension, will make it easier to use than the existing tooling of isort and flake8.

For example, if we wanted to upgrade to (and require) using f-strings across the repo, we would enable a single rule with a single line config change by adding UP031 and UP032 to the select section of the Ruff config. The alternative to this would be to add the pyupgrade pre-commit hook, which is another dependency that would need to be used. Or if we wanted to add type checking to the repo then the ANN ruleset could be enabled alongside mypy to make adding types easier; the alternative to this would be to add an additional flake8 plugin flake8-annotations.

Regarding the referenced issue, the reason for the lack of movement on this relates to the ruff formatter, which would contradict the vertical hanging indent setting in isort. Adding formatting, e.g. black or the ruff formatter, would be a conversation for another time.

[johnraz]

@calumy thanks so much for taking the time to elaborate 🙏

On the CI part, I do agree linting should solely happen in the pre-commit step of the pipeline and should be applied to the all repository. Fully onboard with a separated PR to fix that 👌🏻

I do see the point about ruff being a one size fit all solution but I still kinda trust individual dedicated tool better. As you mentioned there always alternative to add on features. Not blocking this if others are in favour though

[johnraz]

I'm going to close this PR for now - we can re-evaluate later if needed, thanks again for the submission