Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

style: Introduce ruff for linting in all plugins. #354

Merged
merged 17 commits into from
Oct 2, 2023
Merged

style: Introduce ruff for linting in all plugins. #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
437eed1
Use ruff for linting in kedro-telemetry
merelcht Sep 27, 2023
a9ae1f1
Setup ruff in pre-commit
merelcht Sep 27, 2023
cbddb75
Run ruff on telemetry
merelcht Sep 27, 2023
d73dadf
Add ruff to pre-commit for telemetry
merelcht Sep 28, 2023
b6ba480
Add ruff setup to kedro-airflow
merelcht Sep 28, 2023
9e48420
Add ruff setup to kedro-docker
merelcht Sep 28, 2023
f7ac287
Add ruff setup to kedro-datasets
merelcht Sep 28, 2023
d714fa2
Run ruff in make lint command
merelcht Sep 28, 2023
7691ee4
Add ruff to test requirements
merelcht Sep 28, 2023
a5eb0c0
Remove all mentions of pylint
merelcht Sep 28, 2023
a009181
Fix linting in tensorflow dataset
merelcht Sep 28, 2023
b847c20
Run linting on python 3.11
merelcht Sep 28, 2023
c3d2397
Bump pre-commit version
merelcht Sep 28, 2023
a768309
Bump pre-commit version
merelcht Sep 28, 2023
613b7b9
Try bump
merelcht Sep 28, 2023
6750f76
Update bump
merelcht Sep 28, 2023
6ef146b
Remove upperbound precommit
merelcht Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/kedro-airflow.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
with:
plugin: kedro-airflow
os: ubuntu-latest
python-version: "3.8"
python-version: "3.11"

e2e-tests:
strategy:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/kedro-docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
with:
plugin: kedro-docker
os: ubuntu-latest
python-version: "3.8"
python-version: "3.11"

e2e-tests:
strategy:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/kedro-telemetry.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
with:
plugin: kedro-telemetry
os: ubuntu-latest
python-version: "3.8"
python-version: "3.11"

e2e-tests:
strategy:
Expand Down
172 changes: 19 additions & 153 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ default_stages: [commit, manual]

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.4.0
rev: v3.4.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
Expand All @@ -14,178 +14,44 @@ repos:
- id: check-case-conflict # Check for files that would conflict in case-insensitive filesystems
- id: check-merge-conflict # Check for files that contain merge conflict strings.
- id: debug-statements # Check for debugger imports and py37+ `breakpoint()` calls in python source.
- id: flake8
files: ^(kedro-datasets/kedro_datasets/|kedro-airflow/kedro_airflow/|kedro-docker/kedro_docker/|kedro-telemetry/kedro_telemetry/)
args:
- "--max-line-length=88"
- "--max-complexity=18"
- "--select=B,C,E,F,W,T4,B9"
- "--ignore=E203,E266,E501,W503"
exclude: "^kedro_airflow/dag_template.py|^template.py"

- repo: local
hooks:
# pylint quick checks
- id: pylint-quick-kedro-datasets
name: "Quick PyLint on kedro_datasets/*"
- id: ruff-kedro-datasets
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/kedro-org/kedro/blob/af2ba0ed7abaf004da45a443a0b3d37bf11dd30f/.pre-commit-config.yaml#L6C1-L14C69

- repo: https://github.com/astral-sh/ruff-pre-commit
  # Ruff version.
  rev: v0.0.277

Can we use the ruff pre-commit hook? this way we don't have to install ruff locally and ensure CI always work the same as local.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know if it's possible to run it only for the plugin that has been changed with the pre-commit hook. @SajidAlamQB do you know if that could work? I know you put a lot of thought into this setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes @merelcht is correct, the main issue is that we can't dynamically run hooks based on the specific plugin environment on the CI. The setup will trigger all the defined ruff linting hooks on all plugins, regardless of which plugin has changes.

A workaround would be to create a script that checks for changed plugins, and then conditionally executes ruff on those plugins.

name: "Ruff on kedro_datasets/*"
language: system
types: [file, python]
files: ^kedro-datasets/kedro_datasets/
exclude: ^(?!kedro-datasets/kedro_datasets/).*\.py$
entry: pylint --rcfile kedro-datasets/pyproject.toml --disable=unnecessary-pass
stages: [commit]

- id: pylint-quick-kedro-airflow
name: "Quick PyLint on kedro_airflow/*"
language: system
types: [file, python]
files: ^kedro-airflow/kedro_airflow/
exclude: ^(?!kedro-airflow/kedro_airflow/).*\.py$
entry: pylint --disable=unnecessary-pass
stages: [commit]

- id: pylint-quick-kedro-docker
name: "Quick PyLint on kedro_docker/*"
language: system
types: [file, python]
files: ^kedro-docker/kedro_docker/
exclude: ^(?!kedro-docker/kedro_docker/).*\.py$
entry: pylint --disable=unnecessary-pass
stages: [commit]

- id: pylint-quick-kedro-telemetry
name: "Quick PyLint on kedro_telemetry/*"
language: system
types: [file, python]
files: ^kedro-telemetry/kedro_telemetry/
exclude: ^(?!kedro-telemetry/kedro_telemetry/).*\.py$
entry: pylint --disable=unnecessary-pass
stages: [commit]

# pylint full checks
- id: pylint-kedro-datasets
name: "PyLint on kedro_datasets/*"
language: system
files: ^kedro-datasets/kedro_datasets/.*\.py$
exclude: ^(?!kedro-datasets/kedro_datasets/).*\.py$
pass_filenames: false
stages: [manual]
entry: pylint --rcfile kedro-datasets/pyproject.toml --disable=unnecessary-pass,E0401 kedro-datasets/kedro_datasets

- id: pylint-kedro-datasets-features
name: "PyLint on kedro-datasets features/*"
language: system
files: ^kedro-datasets/features/.*\.py$
exclude: ^(?!kedro-datasets/features/).*\.py$
pass_filenames: false
stages: [manual]
entry: pylint --rcfile kedro-datasets/pyproject.toml --disable=missing-docstring,no-name-in-module,E0401 kedro-datasets/features

- id: pylint-kedro-datasets-tests
name: "PyLint on kedro-datasets tests/*"
language: system
files: ^kedro-datasets/tests/.*\.py$
exclude: ^(?!kedro-datasets/tests/).*\.py$
pass_filenames: false
stages: [manual]
entry: pylint --rcfile kedro-datasets/pyproject.toml --disable=missing-docstring,redefined-outer-name,no-self-use,invalid-name,protected-access,too-many-arguments,E0401 kedro-datasets/tests
stages: [ manual ]
entry: ruff kedro-datasets --fix --exit-non-zero-on-fix

- id: pylint-kedro-airflow
name: "PyLint on kedro_airflow/*"
- id: ruff-kedro-airflow
name: "Ruff on kedro_airflow/*"
language: system
files: ^kedro-airflow/kedro_airflow/.*\.py$
files: ^kedro-airflow/kedro_airflow/
exclude: ^(?!kedro-airflow/kedro_airflow/).*\.py$
pass_filenames: false
stages: [manual]
entry: pylint --disable=unnecessary-pass,E0401 kedro-airflow/kedro_airflow

- id: pylint-kedro-airflow-features
name: "PyLint on kedro-airflow features/*"
language: system
pass_filenames: false
stages: [manual]
entry: pylint --disable=missing-docstring,no-name-in-module kedro-airflow/features

- id: pylint-kedro-airflow-tests
name: "PyLint on kedro-airflow tests/*"
language: system
pass_filenames: false
stages: [manual]
entry: pylint --disable=missing-docstring,redefined-outer-name,no-self-use,invalid-name,protected-access,too-many-arguments kedro-airflow/tests
stages: [ manual ]
entry: ruff kedro-airflow --fix --exit-non-zero-on-fix

- id: pylint-kedro-docker
name: "PyLint on kedro_docker/*"
- id: ruff-kedro-docker
name: "Ruff on kedro_docker/*"
language: system
files: ^kedro-docker/kedro_docker/.*\.py$
files: ^kedro-docker/kedro_docker/
exclude: ^(?!kedro-docker/kedro_docker/).*\.py$
pass_filenames: false
stages: [manual]
entry: pylint --disable=unnecessary-pass,E0401 kedro-docker/kedro_docker

- id: pylint-kedro-docker-features
name: "PyLint on kedro-docker features/*"
language: system
pass_filenames: false
stages: [manual]
entry: pylint --disable=missing-docstring,no-name-in-module kedro-docker/features

- id: pylint-kedro-docker-tests
name: "PyLint on kedro-docker tests/*"
language: system
pass_filenames: false
stages: [manual]
entry: pylint --disable=missing-docstring,redefined-outer-name,invalid-name,protected-access,too-many-arguments kedro-docker/tests

- id: pylint-kedro-telemetry
name: "PyLint on kedro_telemetry/*"
language: system
files: ^kedro-telemetry/kedro_telemetry/.*\.py$
exclude: ^(?!kedro-telemetry/kedro_telemetry/).*\.py$
pass_filenames: false
stages: [manual]
entry: pylint --disable=unnecessary-pass,E0401 kedro-telemetry/kedro_telemetry

- id: pylint-kedro-telemetry-features
name: "PyLint on kedro-docker features/*"
language: system
stages: [ manual ]
entry: echo 'Not needed to run for this directory'
files: .*
entry: ruff kedro-docker --fix --exit-non-zero-on-fix

- id: pylint-kedro-telemetry-tests
name: "PyLint on kedro-telemetry tests/*"
- id: ruff-kedro-telemetry
name: "Ruff on kedro_telemetry/*"
language: system
files: ^kedro-telemetry/kedro_telemetry/
exclude: ^(?!kedro-telemetry/kedro_telemetry/).*\.py$
pass_filenames: false
stages: [manual]
entry: pylint --disable=missing-docstring,redefined-outer-name,no-self-use,invalid-name,protected-access,too-many-arguments kedro-telemetry/tests

- id: isort-kedro-datasets
name: "Sort imports"
language: system
types: [ file, python ]
files: ^kedro-datasets/
entry: isort

- id: isort-kedro-docker
name: "Sort imports"
language: system
types: [ file, python ]
files: ^kedro-docker/
entry: isort

- id: isort-kedro-airflow
name: "Sort imports"
language: system
types: [ file, python ]
files: ^kedro-airflow/
entry: isort

- id: isort-kedro-telemetry
name: "Sort imports"
language: system
types: [ file, python ]
files: ^kedro-telemetry/
entry: isort
entry: ruff kedro-telemetry --fix --exit-non-zero-on-fix

- id: black-kedro-datasets
name: "Black"
Expand Down
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ install-pip-setuptools:
python -m pip install -U pip setuptools wheel

lint:
pre-commit run trailing-whitespace --all-files && pre-commit run end-of-file-fixer --all-files && pre-commit run check-yaml --all-files && pre-commit run check-added-large-files --all-files && pre-commit run check-case-conflict --all-files && pre-commit run check-merge-conflict --all-files && pre-commit run debug-statements --all-files && pre-commit run flake8 --all-files && pre-commit run isort-$(plugin) --all-files --hook-stage manual && pre-commit run black-$(plugin) --all-files --hook-stage manual && pre-commit run secret_scan --all-files --hook-stage manual && pre-commit run bandit --all-files --hook-stage manual && pre-commit run pylint-$(plugin) --all-files --hook-stage manual && pre-commit run pylint-$(plugin)-features --all-files --hook-stage manual && pre-commit run pylint-$(plugin)-tests --all-files --hook-stage manual
pre-commit run -a --hook-stage manual ruff-$(plugin) && pre-commit run trailing-whitespace --all-files && pre-commit run end-of-file-fixer --all-files && pre-commit run check-yaml --all-files && pre-commit run check-added-large-files --all-files && pre-commit run check-case-conflict --all-files && pre-commit run check-merge-conflict --all-files && pre-commit run debug-statements --all-files && pre-commit run black-$(plugin) --all-files --hook-stage manual && pre-commit run secret_scan --all-files --hook-stage manual && pre-commit run bandit --all-files --hook-stage manual

test:
cd $(plugin) && pytest tests --cov-config pyproject.toml --numprocesses 4 --dist loadfile
Expand Down
Loading