Introducing ruff as a linter to replace pylint, flake8 and isort #2634

noklam · 2023-06-02T14:04:25Z

Signed-off-by: Nok nok.lam.chan@quantumblack.com

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

The goal for this is to use ruff to replace pylint, we should aim at compatible (99.9%) for this scope. We can decide later rather we should enable extra rule set.

ruff is much fast than pylint, which kills dev productivity for trivial changes. black will be kept because ruff is not a formatter although it can flag issues with formatting.

Development notes

Update linting.md
Remove isort,flake8,pylint from our CI (noted the dependencies are still here because we only deprecate kedro lint in 0.19.0`

Checklist

Read the contributing guidelines
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added a description of this change in the RELEASE.md file
Added tests to cover my changes

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam · 2023-06-02T15:20:51Z

Playing around with it, it's amazingly fast. I get confused a little bit with max-length. It seems that it can only warn the line is too long but couldn't fix it.

So I configure it to ignore E501(line-length), and just run black over it instead. There are subtle difference so they are not 1:1 mapping, but I guess black is doing a good job there and I am not worry about it.

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

…nto replace-pylint Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam · 2023-07-03T17:07:51Z

To test this you can still uncomment the existing setup. They are mostly identical but with some slight difference as ruff is not a 100% replacement to the existing tools.

datajoely · 2023-07-03T17:14:58Z

❤️

astrojuanlu · 2023-07-03T17:33:52Z

This still calls pylint right? Maybe we can remove it from CI to see the difference in execution times

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam · 2023-07-03T23:06:49Z

@astrojuanlu Ruff run almost instantly compare to pylint :) I have removed it from the pre-commit config now.

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

.pre-commit-config.yaml

astrojuanlu · 2023-07-04T08:05:19Z

test_requirements.txt

@@ -20,7 +20,6 @@ holoviews~=1.13.0
 import-linter[toml]==1.8.0
 ipython>=7.31.1, <8.0; python_version < '3.8'
 ipython~=8.10; python_version >= '3.8'
-isort~=5.0
 Jinja2<3.1.0
 joblib>=0.14
 jupyterlab_server>=2.11.1, <2.16.0 # 2.16.0 requires importlib_metedata >= 4.8.3 which conflicts with flake8 requirement


Does this mean that we don't need to cap jupyterlab_server anymore?

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

… of `else` then `if`, to reduce indentation Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam · 2023-07-18T17:32:33Z

There are many linting error, many of them are not difference between ruff and pylint but rather we are using old pylint version.

For the changes that are obvious and meaningful, I have applied the changes and suppressed the rest.

For example, this is a good fix hinted by ruff e0275f0.

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

sbrugman · 2023-07-18T17:14:54Z

.pre-commit-config.yaml

+          name: "ruff on tests/ and docs/"
+          # PLR2004: Magic value used
+          # PLR0913: Too many arguments
+          args: ["--fix", "--exit-non-zero-on-fix", "--ignore=PLR2004,PLR0913"]


Ruff allows you to have different sets of rules for these cases. Run the linter once, and select/ignore rules for testing.

For an example, check GitHub.com/ing-bank/popmon

@sbrugman Could you share a link? Not sure if I missed anything, I checked their config but it does not use any arguments.

https://github.com/ing-bank/popmon/blob/ac79d212a519368d01525950142e0a282f5287c3/.pre-commit-config.yaml#L6-L10

See the section [tool.ruff.per-file-ignores] in pyproject.toml

.pre-commit-config.yaml

docs/source/development/linting.md

kedro/extras/datasets/geopandas/geojson_dataset.py

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

merelcht

Great work @noklam ⭐ I went through most of the changed files and can see that the changes minimal and mostly just changing # pylint: disable to # noqa.

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam · 2023-07-20T16:24:30Z

I don't want this PR stuck forever, since this is already very useful to speed up dev.

Follows up:

See the section [tool.ruff.per-file-ignores] in pyproject.toml
Replace bandit with ruff

astrojuanlu · 2023-07-20T16:46:12Z

I agree it would be super cool to get this in ASAP. Plugins can follow suit.

See the section [tool.ruff.per-file-ignores] in pyproject.toml

Just to clarify, is that something you want to do in this PR for the CI to pass?

noklam · 2023-07-20T17:11:49Z

No, I will save this for future PRs.

The lining is still failing for weird reason, I cannot reproduce it anywhere so I may have to ssh into it.

Other than that it's block by the E2e test.

noklam · 2023-07-20T22:17:52Z

The unsort import error is mysteriou, I cannot reproduce it anywhere. It's not important at all so I silent this error for the micropkg.py.

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam · 2023-07-20T22:48:22Z

Linting now all pass, only blocked by #2816, enable auto-merge.

first go

9e9babd

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam added 2 commits June 2, 2023 15:35

Add isort rules

61f49a9

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

Merge branch 'main' into replace-pylint

871ce32

noklam changed the title ~~Draft - remove pylint~~ Draft - remove pylint with ruff Jul 3, 2023

noklam added Help Wanted 🙏 Contribution task, outside help would be appreciated! Community Issue/PR opened by the open-source community labels Jul 3, 2023

noklam added 6 commits July 3, 2023 13:14

Merge branch 'main' into replace-pylint

0126fa7

update isort setting

aba03f5

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

Merge branch 'main' into replace-pylint

8c851d5

Add comments for pyproject ruff settings

b288caa

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

update pre-commit setting

a28d2a0

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

Merge branch 'replace-pylint' of https://github.com/kedro-org/kedro i…

155d710

…nto replace-pylint Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam changed the title ~~Draft - remove pylint with ruff~~ Introducing ruff as a linter and replace pylint, flake8 and isort Jul 3, 2023

noklam self-assigned this Jul 3, 2023

noklam added this to the DevOps and DevSetup autumn cleanup milestone Jul 3, 2023

noklam requested review from astrojuanlu and deepyaman July 3, 2023 17:05

noklam marked this pull request as ready for review July 3, 2023 17:05

noklam requested a review from idanov as a code owner July 3, 2023 17:05

noklam changed the title ~~Introducing ruff as a linter and replace pylint, flake8 and isort~~ Introducing ruff as a linter to replace pylint, flake8 and isort Jul 3, 2023

noklam added 2 commits July 3, 2023 23:04

remove pylint from pre-commit

37c515f

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

remove linting library from test requirements

12d6017

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

fix bash script to skip pylint isort

d08151d

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

astrojuanlu reviewed Jul 4, 2023

View reviewed changes

.pre-commit-config.yaml Show resolved Hide resolved

astrojuanlu reviewed Jul 4, 2023

View reviewed changes

noklam added 4 commits July 18, 2023 17:21

PLR0913 - Too many arguments to function call

f6cf481

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

PLR2004 - magic value

14a51e4

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

linting

13f5c39

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

kedro/framework/context/context.py:151:13: PLR5501 Use elif instead…

e0275f0

… of `else` then `if`, to reduce indentation Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam marked this pull request as ready for review July 18, 2023 17:33

noklam requested review from merelcht and stichbury and removed request for stichbury July 18, 2023 17:34

noklam marked this pull request as draft July 18, 2023 17:42

fix lint

6903213

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam marked this pull request as ready for review July 18, 2023 17:45

sbrugman reviewed Jul 18, 2023

View reviewed changes

noklam added 3 commits July 18, 2023 23:12

fix doc links

abc9c5c

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

Update config to show-fix for debugging

a159d03

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

Fix ruff args

49589af

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

merelcht approved these changes Jul 20, 2023

View reviewed changes

noklam added 2 commits July 20, 2023 13:41

Update ruff version

d9948b5

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

Update ruff versions

0fc6c84

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

Suppress sort import error for linting

0a13f55

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>

noklam enabled auto-merge (squash) July 20, 2023 22:48

Merge branch 'main' into replace-pylint

ab98200

noklam merged commit 638c01b into main Jul 26, 2023

noklam deleted the replace-pylint branch July 26, 2023 18:56

rashidakanchwala mentioned this pull request Dec 6, 2023

Introducing ruff as a linter to replace pylint, flake8, isort and black. kedro-org/kedro-viz#1670

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing ruff as a linter to replace pylint, flake8 and isort #2634

Introducing ruff as a linter to replace pylint, flake8 and isort #2634

noklam commented Jun 2, 2023 •

edited

Loading

noklam commented Jun 2, 2023

noklam commented Jul 3, 2023

datajoely commented Jul 3, 2023

astrojuanlu commented Jul 3, 2023

noklam commented Jul 3, 2023 •

edited

Loading

astrojuanlu Jul 4, 2023

astrojuanlu Jul 4, 2023

noklam commented Jul 18, 2023

sbrugman Jul 18, 2023

noklam Jul 18, 2023

sbrugman Jul 19, 2023

merelcht left a comment

noklam commented Jul 20, 2023

astrojuanlu commented Jul 20, 2023

noklam commented Jul 20, 2023

noklam commented Jul 20, 2023

noklam commented Jul 20, 2023

Introducing ruff as a linter to replace pylint, flake8 and isort #2634

Introducing ruff as a linter to replace pylint, flake8 and isort #2634

Conversation

noklam commented Jun 2, 2023 • edited Loading

Description

Development notes

Checklist

noklam commented Jun 2, 2023

noklam commented Jul 3, 2023

datajoely commented Jul 3, 2023

astrojuanlu commented Jul 3, 2023

noklam commented Jul 3, 2023 • edited Loading

astrojuanlu Jul 4, 2023

Choose a reason for hiding this comment

astrojuanlu Jul 4, 2023

Choose a reason for hiding this comment

noklam commented Jul 18, 2023

sbrugman Jul 18, 2023

Choose a reason for hiding this comment

noklam Jul 18, 2023

Choose a reason for hiding this comment

sbrugman Jul 19, 2023

Choose a reason for hiding this comment

merelcht left a comment

Choose a reason for hiding this comment

noklam commented Jul 20, 2023

astrojuanlu commented Jul 20, 2023

noklam commented Jul 20, 2023

noklam commented Jul 20, 2023

noklam commented Jul 20, 2023

noklam commented Jun 2, 2023 •

edited

Loading

noklam commented Jul 3, 2023 •

edited

Loading