Skip to content
This repository was archived by the owner on Sep 2, 2024. It is now read-only.

Idea: use ruff as a replacement for flake8 + isort ?#890

Merged
Tom-Willemsen merged 2 commits into
mainfrom
use_ruff
Sep 18, 2023
Merged

Idea: use ruff as a replacement for flake8 + isort ?#890
Tom-Willemsen merged 2 commits into
mainfrom
use_ruff

Conversation

@Tom-Willemsen

@Tom-Willemsen Tom-Willemsen commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

Test for adding ruff as a replacement for flake8 + isort.

Main advantage is speed (approx 100x faster):

$ time isort --check .

real	0m0.420s
user	0m0.349s
sys	0m0.040s

$ time flake8 src/

real	0m0.859s
user	0m2.157s
sys	0m0.154s

$ time ruff .

real	0m0.011s
user	0m0.006s
sys	0m0.006s

One code change is required due to a difference in opinion between ruff and isort about constructs like:

from somewhere import thing1
from somewhere import thing2 as aliased_name
from somewhere import thing3

which ruff prefers formatted as:

from somewhere import thing1, thing3
from somewhere import thing2 as aliased_name

Other than that it is a 1:1 replacement in terms of linting rules.

This seems to be the direction a lot of modern python projects are moving in, and a better dev experience with pre-commit etc is always nice.

@Tom-Willemsen Tom-Willemsen changed the title Use ruff as a replacement for flake8 + isort ? Idea: use ruff as a replacement for flake8 + isort ? Sep 11, 2023
@codecov

codecov Bot commented Sep 11, 2023

Copy link
Copy Markdown

Codecov Report

Merging #890 (93649a2) into main (5e751e8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #890   +/-   ##
=======================================
  Coverage   92.96%   92.96%           
=======================================
  Files          50       50           
  Lines        2417     2417           
=======================================
  Hits         2247     2247           
  Misses        170      170           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This worked for me but required a bit of faffing to get vscode happy. Had to change Python: Select Linter to None and install the ruff extension. @dperl-dls, do you have strong opinions on this? One issue is that we're now using a different formatter to dodal.

@Tom-Willemsen

Copy link
Copy Markdown
Contributor Author

One issue is that we're now using a different formatter to dodal.

I agree if we decide to use this we should also do an equivalent migration on dodal - I only opened the one PR initially in order to get opinions and not waste too much time if we decide we don't like this.

@DominicOram

Copy link
Copy Markdown
Collaborator

One issue is that we're now using a different formatter to dodal.

I agree if we decide to use this we should also do an equivalent migration on dodal - I only opened the one PR initially in order to get opinions and not waste too much time if we decide we don't like this.

In which case we should probably ask @callumforrester

@d-perl

d-perl commented Sep 12, 2023

Copy link
Copy Markdown
Contributor

I don't have a very strong opinion on this but I have noticed some slowdown with flake8 so I am mildly in favour

@callumforrester

Copy link
Copy Markdown

@coretl, weren't you looking at migrating the skeleton to ruff at some point?

@coretl

coretl commented Sep 12, 2023

Copy link
Copy Markdown

we are planning to, but haven't had time yet: DiamondLightSource/python3-pip-skeleton#142

@DominicOram

Copy link
Copy Markdown
Collaborator

Is a better way to do this to change the skeleton then merge it in here and dodal then?

@coretl

coretl commented Sep 12, 2023

Copy link
Copy Markdown

@Tom-Willemsen just volunteered to do this and I said yes please...

@Tom-Willemsen

Copy link
Copy Markdown
Contributor Author

@Tom-Willemsen

Copy link
Copy Markdown
Contributor Author

This PR is now a cherry-pick/merge of DiamondLightSource/python3-pip-skeleton#147 .

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me, thanks. Can we merge it into dodal too please?

@Tom-Willemsen
Tom-Willemsen merged commit 0b10653 into main Sep 18, 2023
@Tom-Willemsen
Tom-Willemsen deleted the use_ruff branch September 18, 2023 12:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants