DiamondLightSource / python3-pip-skeleton

Archived in favour of https://github.com/DiamondLightSource/python-copier-template
Apache License 2.0
4 stars 4 forks source link

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

Closed Tom-Willemsen closed 1 year ago

Tom-Willemsen commented 1 year ago

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:

gilesknap commented 1 year ago

@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?

codecov[bot] commented 1 year ago

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           

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

gilesknap commented 1 year ago

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 commented 1 year ago

This seems fine, shall I merge?

Tom-Willemsen commented 1 year ago

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