Skip to content

Commit

Permalink
Introducing ruff as a linter to replace pylint, flake8 and isort (#2634)
Browse files Browse the repository at this point in the history
* first go

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

* Add isort rules

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

* update isort setting

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

* Add comments for pyproject ruff settings

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

* update pre-commit setting

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

* remove pylint from pre-commit

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

* remove linting library from test requirements

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

* fix bash script to skip pylint isort

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

* Update menitions to pylint, isort and flake8

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

* Remove flake8 plugin section in the manifest

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

* First draft to update linting docs

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

* Apply suggestions from code review

Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>

* Fix subprocess.pipe with ruff recommendation

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

* Fix test asserting stdout,stderr to use capture_output=True

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

* fix more docs

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

* Add pylint rule sets and replace pylint disable with noqa

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

* Fix according to ruff, merge isinstance check PLR1701 Merge these isinstance calls: `isinstance(app.builder, (CheckExternalLinksBuilder, LaTeXBuilder))`

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

* fix test according to lint: tests/framework/session/test_session.py:588:38: PLC1901 `exception["value"] == ""` can be simplified to `not exception["value"]` as an empty string is falsey

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

* fix dataset linting

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

* fix config linting

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

* temporary commit for merge

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

* remove unused command in makefile

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

* Fix linting for runner

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

* fix micropkg lint

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

* Fix PLW2901 # noqa: redefined-loop-name

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

* PLR0913 - Too many arguments to function call

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

* PLR2004 - magic value

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

* linting

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

* kedro/framework/context/context.py:151:13: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation

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

* fix lint

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

* fix doc links

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

* Update config to show-fix for debugging

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

* Fix ruff args

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

* Update ruff version

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

* Update ruff versions

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

* Suppress sort import error for linting

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

---------

Signed-off-by: Nok <nok.lam.chan@quantumblack.com>
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
  • Loading branch information
noklam and merelcht authored Jul 26, 2023
1 parent c74114c commit 638c01b
Show file tree
Hide file tree
Showing 92 changed files with 322 additions and 377 deletions.
111 changes: 42 additions & 69 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,57 @@ repos:
exclude: "^kedro/templates/|^features/steps/test_starter/"
- id: requirements-txt-fixer # Sorts entries in requirements.txt
exclude: "^kedro/templates/|^features/steps/test_starter/"

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.961
hooks:
- id: mypy
args: [--allow-redefinition, --ignore-missing-imports]
exclude: |
(?x)(
^kedro/templates/|
^docs/|
^features/steps/test_starter/
)
additional_dependencies:
- types-cachetools
- types-filelock
- types-PyYAML
- types-redis
- types-requests
- types-setuptools
- types-toml
- attrs

- repo: https://github.com/asottile/blacken-docs
rev: v1.12.1
hooks:
- id: blacken-docs
additional_dependencies: [black~=22.0]
entry: blacken-docs --skip-errors

- repo: https://github.com/asottile/pyupgrade
rev: v2.26.0
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.0.277
hooks:
- id: ruff
name: "ruff on kedro/"
args: ["--fix", "--show-fixes", "--exit-non-zero-on-fix"]
exclude: "^kedro/templates/|^features/steps/test_starter/|tests|docs"
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.0.277
hooks:
- id: pyupgrade
args: [--py36-plus]
- id: ruff
name: "ruff on tests/ and docs/"
# PLR2004: Magic value used
# PLR0913: Too many arguments
args: ["--fix", "--show-fixes", "--exit-non-zero-on-fix", "--ignore=PLR2004,PLR0913"]
# include: "tests"
exclude: "^kedro/templates/|^features/steps/test_starter/|kedro"

- repo: local
hooks:
- id: isort
name: "Sort imports"
language: system
types: [file, python]
exclude: ^kedro/templates/|^features/steps/test_starter
entry: isort
- id: black
name: "Black"
language: system
Expand All @@ -53,65 +83,6 @@ repos:
pass_filenames: false
entry: lint-imports


# It's impossible to specify per-directory configuration, so we just run it many times.
# https://github.com/PyCQA/pylint/issues/618
# The first set of pylint checks if for local pre-commit, it only runs on the files changed.
- id: pylint-quick-kedro
name: "Quick Pylint on kedro/*"
language: system
types: [file, python]
files: ^kedro/
exclude: ^kedro/templates/
entry: pylint -j 4 --disable=unnecessary-pass
stages: [commit]
- id: pylint-quick-tests
name: "Quick Pylint on tests/*"
language: system
types: [file, python]
files: ^tests/
entry: pylint -j 4 --disable=missing-docstring,redefined-outer-name,no-self-use,invalid-name,protected-access,too-many-arguments,too-many-public-methods
stages: [commit]
- id: pylint-quick-features
name: "Quick Pylint on features/*"
language: system
types: [file, python]
files: ^features/
exclude: ^features/steps/test_starter
entry: pylint -j 4 --disable=missing-docstring,no-name-in-module
stages: [commit]

# The same pylint checks, but running on all files. It's for manual run with `make lint`
- id: pylint-kedro
name: "Pylint on kedro/*"
language: system
pass_filenames: false
stages: [manual]
entry: pylint -j 4 --disable=unnecessary-pass --init-hook="import sys; sys.setrecursionlimit(2000)" kedro
- id: pylint-tests
name: "Pylint on tests/*"
language: system
pass_filenames: false
stages: [manual]
entry: pylint -j 4 --disable=missing-docstring,redefined-outer-name,no-self-use,invalid-name,protected-access,too-many-arguments,too-many-public-methods,use-implicit-booleaness-not-comparison tests
- id: pylint-features
name: "Pylint on features/*"
language: system
pass_filenames: false
stages: [manual]
exclude: ^features/steps/test_starter
entry: pylint -j 4 --disable=missing-docstring,no-name-in-module features
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.2.3
hooks:
- id: flake8
args:
- "--max-line-length=88"
- "--max-complexity=18"
- "--max-complexity=18"
- "--select=B,C,E,F,W,T4,B9"
- "--ignore=E203,E266,E501,W503"
exclude: "^kedro/templates/|^features/steps/test_starter/"
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.961
hooks:
Expand Down Expand Up @@ -147,3 +118,5 @@ repos:
types: [file, python]
exclude: ^kedro/templates/|^tests/|^features/steps/test_starter
entry: bandit -ll

# Manual only
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ install-pip-setuptools:

lint:
pre-commit run -a --hook-stage manual $(hook)

test:
pytest --numprocesses 4 --dist loadfile

Expand Down
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* Pin `pip<23.2` for CI due to a breaking change. See https://github.com/kedro-org/kedro/pull/2813

## Documentation changes
- Recommended `ruff` as the linter and remove mentions of `pylint`, `isort`, `flake8`.

## Breaking changes to the API

Expand Down
3 changes: 1 addition & 2 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,7 @@ def _add_jinja_filters(app):
# LaTeXBuilder is used in the PDF docs build,
# and it doesn't have attribute 'templates'
if not (
isinstance(app.builder, LaTeXBuilder)
or isinstance(app.builder, CheckExternalLinksBuilder)
isinstance(app.builder, (LaTeXBuilder,CheckExternalLinksBuilder))
):
app.builder.templates.environment.filters["env_override"] = env_override

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ We will work with you to complete your contribution, but we reserve the right to
```

Ensure that your PR builds cleanly before you submit it, by running the CI/CD checks locally, as follows:
* `make lint`: PEP-8 Standards (`pylint`, `flake8`)
* `make lint`: PEP-8 Standards (`ruff`, `black`)
* `make test`: unit tests, 100% coverage (`pytest`, `pytest-cov`)
* `make e2e-tests`: end-to-end tests (`behave`)

Expand Down
2 changes: 1 addition & 1 deletion docs/source/development/commands_reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ The `build-docs` command builds [project documentation](../tutorial/package_a_pr
#### Lint your project

```{note}
_This command will be deprecated from Kedro version 0.19.0._
_This command will be deprecated from Kedro version 0.19.0._. We still recommend to (../development/linting.md) and you can find more help here
```

```bash
Expand Down
57 changes: 34 additions & 23 deletions docs/source/development/linting.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,23 @@ As a project grows and goes through various stages of development it becomes imp

## Set up Python tools
There are a variety of Python tools available to use with your Kedro projects. This guide shows you how to use
[`black`](https://github.com/psf/black), [`flake8`](https://github.com/PyCQA/flake8), and
[`isort`](https://github.com/PyCQA/isort) to format and lint your Kedro projects.
[`black`](https://github.com/psf/black), [`ruff`](https://beta.ruff.rs).
- **`black`** is a [PEP 8](https://peps.python.org/pep-0008/) compliant opinionated Python code formatter. `black` can
check for styling inconsistencies and reformat your files in place.
[You can read more in the `black` documentation](https://black.readthedocs.io/en/stable/).
- **`flake8`** is a wrapper around [`pep8`](https://pypi.org/project/pep8/),
[`pyflakes`](https://pypi.org/project/pyflakes/), and [`mccabe`](https://pypi.org/project/mccabe/) which can lint code and format it with respect to [PEP 8](https://peps.python.org/pep-0008/),
and check the [cyclomatic complexity](https://www.ibm.com/docs/en/raa/6.1?topic=metrics-cyclomatic-complexity) of your code base.
[You can read more in the `flake8` documentation](https://flake8.pycqa.org/en/latest/).
- **`isort`** is a Python library used to reformat code by sorting imports alphabetically and automatically separating them into sections by
- **`ruff`** is a fast linter that replaces `flake8`, `pylint`, `pyupgrade`, `isort` and [more](https://beta.ruff.rs/docs/rules/).
- It helps to make your code compliant to [`pep8`](https://pypi.org/project/pep8/).
- It reformats code by sorting imports alphabetically and automatically separating them into sections by
type. [You can read more in the `isort` documentation](https://pycqa.github.io/isort/).


### Install the tools
Install `black`, `flake8`, and `isort` by adding the following lines to your project's `src/requirements.txt`
Install `black` and `ruff` by adding the following lines to your project's `src/requirements.txt`
file:
```text
black # Used for formatting code
flake8 # Used for linting and formatting
isort # Used for formatting code (sorting module imports)
ruff # Used for linting, formatting and sorting module imports
```
To install all the project-specific dependencies, including the linting tools, navigate to the root directory of the
project and run:
Expand All @@ -37,10 +35,29 @@ pip install -r src/requirements.txt
```
Alternatively, you can individually install the linting tools using the following shell commands:
```bash
pip install black
pip install flake8
pip install isort
pip install black ruff
```
#### Configure `ruff`
`ruff` read configurations from `pyproject.toml` within your project root. You can enable different rule sets within the `[tool.ruff]` section. For example, the rule set `F` is equivalent to `Pyflakes`.

To start with `ruff`, we recommend adding this section to enable a few basic rules sets.
```toml
[tool.ruff]
select = [
"F", # Pyflakes
"E", # Pycodestyle
"W", # Pycodestyle
"UP", # pyupgrade
"I", # isort
"PL", # Pylint
]
ignore = ["E501"] # Black take care off line-too-long
```

```{note}
It is a good practice to [split your line when it is too long](https://beta.ruff.rs/docs/rules/line-too-long/), so it can be read easily even in a small screen. `ruff` treats this slightly different from `black`, when using together we recommend to disable this rule, i.e. `E501` to avoid conflicts.
```

#### Configure `flake8`

Store your `flake8` configuration in a file named `setup.cfg` within your project root. The Kedro starters use the [following configuration](https://github.com/kedro-org/kedro-starters/blob/main/pandas-iris/%7B%7B%20cookiecutter.repo_name%20%7D%7D/setup.cfg):
Expand Down Expand Up @@ -87,17 +104,11 @@ you want to run before each `commit`.
Below is a sample `YAML` file with entries for `black`,`flake8`, and `isort`:
```yaml
repos:
- repo: https://github.com/pycqa/isort
rev: 5.10.1
hooks:
- id: isort
name: isort (python)
args: ["--profile", "black"]

- repo: https://github.com/pycqa/flake8
rev: '' # pick a git hash / tag to point to
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.0.270
hooks:
- id: flake8
- id: ruff

- repo: https://github.com/psf/black
rev: 22.8.0
Expand Down
2 changes: 1 addition & 1 deletion docs/source/kedro_project_setup/starters.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ Here is the layout of the project as a Cookiecutter template:
├── docs # Project documentation
├── notebooks # Project related Jupyter notebooks (can be used for experimental code before moving the code to src)
├── README.md # Project README
├── setup.cfg # Configuration options for tools e.g. `pytest` or `flake8`
├── setup.cfg # Configuration options for tools e.g. `pytest` or `black`
└── src # Project source code
└── {{ cookiecutter.python_package }}
├── __init.py__
Expand Down
2 changes: 1 addition & 1 deletion features/environment.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""Behave environment setup commands."""
# pylint: disable=unused-argument
# noqa: unused-argument
from __future__ import annotations

import os
Expand Down
6 changes: 3 additions & 3 deletions features/steps/cli_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def add_test_jupyter_nb(context):
"""Create a test jupyter notebook using TEST_JUPYTER_ORG."""
with open(
str(context.root_project_dir / "notebooks" / "hello_world.ipynb"),
"wt",
"w",
encoding="utf-8",
) as test_nb_fh:
test_nb_fh.write(TEST_JUPYTER_ORG)
Expand Down Expand Up @@ -366,7 +366,7 @@ def simulate_nb_execution(context):
"""
with open(
str(context.root_project_dir / "notebooks" / "hello_world.ipynb"),
"wt",
"w",
encoding="utf-8",
) as test_nb_fh:
test_nb_fh.write(TEST_JUPYTER_AFTER_EXEC)
Expand Down Expand Up @@ -554,7 +554,7 @@ def check_additional_cell_added(context):
encoding="utf-8",
) as test_nb_fh:
context.nb_data = json.load(test_nb_fh)
assert len(context.nb_data["cells"]) == 2
assert len(context.nb_data["cells"]) == 2 # noqa: PLR2004


@then("the output should be empty in all the cells in the jupyter notebook")
Expand Down
4 changes: 1 addition & 3 deletions features/steps/sh_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ def run(
if isinstance(cmd, str) and split:
cmd = shlex.split(cmd)
# pylint: disable=subprocess-run-check
result = subprocess.run(
cmd, input="", stdout=subprocess.PIPE, stderr=subprocess.PIPE, **kwargs
)
result = subprocess.run(cmd, input="", capture_output=True, **kwargs)
result.stdout = result.stdout.decode("utf-8")
result.stderr = result.stderr.decode("utf-8")
if print_output:
Expand Down
4 changes: 1 addition & 3 deletions features/steps/test_plugin/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@

class MyPluginHook:
@hook_impl
def after_catalog_created(
self, catalog
): # pylint: disable=unused-argument, no-self-use
def after_catalog_created(self, catalog): # noqa: unused-argument, no-self-use
logger.info("Reached after_catalog_created hook")


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Delete this when you start working on your own Kedro project.
"""
# pylint: disable=invalid-name
# noqa: invalid-name
from __future__ import annotations

import logging
Expand Down
4 changes: 2 additions & 2 deletions kedro/config/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def _load_config_file(
Parsed configuration.
"""
# for performance reasons
import anyconfig # pylint: disable=import-outside-toplevel
import anyconfig # noqa: import-outside-toplevel

try:
# Default to UTF-8, which is Python 3 default encoding, to decode the file
Expand Down Expand Up @@ -230,7 +230,7 @@ def _check_duplicate_keys(

if overlapping_keys:
sorted_keys = ", ".join(sorted(overlapping_keys))
if len(sorted_keys) > 100:
if len(sorted_keys) > 100: # noqa: PLR2004
sorted_keys = sorted_keys[:100] + "..."
duplicates.append(f"{processed_file}: {sorted_keys}")

Expand Down
2 changes: 1 addition & 1 deletion kedro/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class ConfigLoader(AbstractConfigLoader):
"""

def __init__(
def __init__( # noqa: too-many-arguments
self,
conf_source: str,
env: str = None,
Expand Down
Loading

0 comments on commit 638c01b

Please sign in to comment.