Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Use ruff as a linter as a replacement for flake8/isort #147

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

Tom-Willemsen
Copy link
Contributor

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

Use ruff as a linter, as a replacement for flake8 & isort.

ruff is much faster (Somewhere around 100x) than flake8+isort, and seems to be the direction which a lot of modern python projects are taking. It is pretty much 1:1 compatible with flake8 + isort, with very few exceptions.

To test this:

  • Run python -m pip install -e .[dev] or an equivalent command to get ruff installed.
  • Run pre-commit or ruff ., verify that ruff runs successfully
  • Introduce a linting fail in a .py file in src or tests, verify that ruff picks this up
    • running ruff .
    • running pre-commit
  • Launching vscode from this directory, verify that:
    • Ruff lints are reported (but not auto-fixed by default) in editor window. ruff can fix quite a lot of problems automatically but I found this somewhat surprising to run "on save", happy to hear other opinions about this?
    • Ruff automatically changes import order on save.
    • Note: may need to accept installing suggested extensions on first load of vscode on this branch.
  • Verify that relevant docs have been updated to mention ruff instead of flake8 or isort.

@gilesknap
Copy link
Contributor

@Tom-Willemsen looks good. I'll try this out against the ibek project.

@coretl I'm pretty sure we want to go with this - what do you think?

Copy link
Contributor

@coretl coretl left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for doing this! Just a couple of questions...

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #147 (199f346) into main (f02b12e) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #147   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           16        16           
=========================================
  Hits            16        16           

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

pyproject.toml Outdated Show resolved Hide resolved
@gilesknap
Copy link
Contributor

I have adopted the PR branch into ibek and all looks well.

ruff converted an 'unecesary generator to a set comprehension which was nice! :-)
image

One issue I had with docs is probably just because of Pydantic. See below.

I find that the conf.py

autoclass_content = "both"

results in lots of errors like the following

                                                                                                                                                                                                             
/scratch/hgv27681/work/ibek/src/ibek/globals.py:docstring of ibek.globals.BaseSettings:5: WARNING: 'any' reference target not found: ValidationError
/scratch/hgv27681/work/ibek/src/ibek/globals.py:docstring of ibek.globals.BaseSettings:8: WARNING: 'any' reference target not found: __init__
/scratch/hgv27681/work/ibek/src/ibek/globals.py:docstring of ibek.globals.BaseSettings:8: WARNING: 'any' reference target not found: __pydantic_self__
/scratch/hgv27681/work/ibek/src/ibek/globals.py:docstring of ibek.globals.BaseSettings:8: WARNING: 'any' reference target not found: self
/scratch/hgv27681/work/ibek/src/ibek/globals.py:docstring of ibek.globals.BaseSettings:8: WARNING: 'any' reference target not found: self
/scratch/hgv27681/work/ibek/src/ibek/support.py:docstring of ibek.support.Arg:5: WARNING: 'any' reference target not found: ValidationError
/scratch/hgv27681/work/ibek/src/ibek/support.py:docstring of ibek.support.Arg:8: WARNING: 'any' reference target not found: __init__
/scratch/hgv27681/work/ibek/src/ibek/support.py:docstring of ibek.support.Arg:8: WARNING: 'any' reference target not found: __pydantic_self__
/scratch/hgv27681/work/ibek/src/ibek/support.py:docstring of ibek.support.Arg:8: WARNING: 'any' reference target not found: self
/scratch/hgv27681/work/ibek/src/ibek/support.py:docstring of ibek.support.Arg:8: WARNING: 'any' reference target not found: self

restoring

autoclass_content = "clas"

gets rid of these.

Pydantic seems to upset Sphinx quite a bit, so I don't see this as an issue.

@coretl
Copy link
Contributor

coretl commented Sep 15, 2023

This seems fine, shall I merge?

@Tom-Willemsen
Copy link
Contributor Author

If you're happy with it then yes please? Think I've addressed all the review comments that have been left.

@coretl coretl merged commit e150491 into DiamondLightSource:main Sep 15, 2023
12 checks passed
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