Skip to content

Commit 560c66b

Browse files
authored
Run linters using pre-commit (#965)
This change adds a pre-commit hook to run on each commit, that will automatically run code linters and not allow you to commit changes if the linters fail. This is similar to how cudf uses pre-commit, as described in rapidsai/cudf#9309 . The pre-commit checks will also be run in CI as part of the check style script. This change also adds several new linters that weren't being run in CI before: * pydocstyle * mypy * black * isort * cmake-format * cmake-lint Of these new linters - mypy caught an issue where the test_comms.py would always be skipped (`python/raft-dask/raft_dask/test/test_comms.py:23: error: Module "raft_dask" has no attribute "Comms"` ), and Pydocstyle caught some minor formatting inconsistencies in the python docstrings. black/isort ensure consistent code formatting for the python raft code Authors: - Ben Frederickson (https://github.com/benfred) Approvers: - Micka (https://github.com/lowener) - Corey J. Nolet (https://github.com/cjnolet) - Ray Douglass (https://github.com/raydouglass) - Bradley Dice (https://github.com/bdice) URL: #965
1 parent 7176d94 commit 560c66b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+1835
-1372
lines changed

.pre-commit-config.yaml

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
# Copyright (c) 2022, NVIDIA CORPORATION.
2+
3+
repos:
4+
- repo: https://github.com/PyCQA/isort
5+
rev: 5.10.1
6+
hooks:
7+
- id: isort
8+
# Use the config file specific to each subproject so that each
9+
# project can specify its own first/third-party packages.
10+
args: ["--config-root=python/", "--resolve-all-configs"]
11+
files: python/.*
12+
types_or: [python, cython, pyi]
13+
- repo: https://github.com/psf/black
14+
rev: 22.3.0
15+
hooks:
16+
- id: black
17+
files: python/.*
18+
# Explicitly specify the pyproject.toml at the repo root, not per-project.
19+
args: ["--config", "pyproject.toml"]
20+
exclude: .*_version.py,.*versioneer.py
21+
- repo: https://github.com/PyCQA/flake8
22+
rev: 5.0.4
23+
hooks:
24+
- id: flake8
25+
args: ["--config=setup.cfg"]
26+
files: python/.*$
27+
types: [file]
28+
types_or: [python, cython]
29+
additional_dependencies: ["flake8-force"]
30+
- repo: https://github.com/pre-commit/mirrors-mypy
31+
rev: 'v0.971'
32+
hooks:
33+
- id: mypy
34+
additional_dependencies: [types-cachetools]
35+
args: ["--config-file=setup.cfg",
36+
"python/pylibraft/pylibraft",
37+
"python/raft-dask/raft_dask"]
38+
pass_filenames: false
39+
exclude: .*_version.py
40+
- repo: https://github.com/PyCQA/pydocstyle
41+
rev: 6.1.1
42+
hooks:
43+
- id: pydocstyle
44+
args: ["--config=setup.cfg"]
45+
- repo: https://github.com/pre-commit/mirrors-clang-format
46+
rev: v11.1.0
47+
hooks:
48+
- id: clang-format
49+
types_or: [c, c++, cuda]
50+
args: ["-fallback-style=none", "-style=file", "-i"]
51+
exclude: cpp/include/raft/thirdparty/.*
52+
- repo: local
53+
hooks:
54+
- id: no-deprecationwarning
55+
name: no-deprecationwarning
56+
description: 'Enforce that DeprecationWarning is not introduced (use FutureWarning instead)'
57+
entry: '(category=|\s)DeprecationWarning[,)]'
58+
language: pygrep
59+
types_or: [python, cython]
60+
- id: cmake-format
61+
name: cmake-format
62+
entry: ./cpp/scripts/run-cmake-format.sh cmake-format
63+
language: python
64+
types: [cmake]
65+
exclude: .*/thirdparty/.*
66+
# Note that pre-commit autoupdate does not update the versions
67+
# of dependencies, so we'll have to update this manually.
68+
additional_dependencies:
69+
- cmakelang==0.6.13
70+
verbose: true
71+
require_serial: true
72+
- id: cmake-lint
73+
name: cmake-lint
74+
entry: ./cpp/scripts/run-cmake-format.sh cmake-lint
75+
language: python
76+
types: [cmake]
77+
# Note that pre-commit autoupdate does not update the versions
78+
# of dependencies, so we'll have to update this manually.
79+
additional_dependencies:
80+
- cmakelang==0.6.13
81+
verbose: true
82+
require_serial: true
83+
exclude: .*/thirdparty/.*
84+
- id: copyright-check
85+
name: copyright-check
86+
entry: python ./ci/checks/copyright.py --git-modified-only --update-current-year
87+
language: python
88+
pass_filenames: false
89+
additional_dependencies: [gitpython]
90+
- id: include-check
91+
name: include-check
92+
entry: python ./cpp/scripts/include_checker.py cpp/bench cpp/include cpp/test
93+
pass_filenames: false
94+
language: python
95+
additional_dependencies: [gitpython]
96+
97+
default_language_version:
98+
python: python3

CONTRIBUTING.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,43 @@ into three categories:
3737
Remember, if you are unsure about anything, don't hesitate to comment on issues
3838
and ask for clarifications!
3939

40+
41+
### Python / Pre-commit hooks
42+
43+
RAFT uses [pre-commit](https://pre-commit.com/) to execute code linters and formatters such as
44+
[Black](https://black.readthedocs.io/en/stable/), [isort](https://pycqa.github.io/isort/), and
45+
[flake8](https://flake8.pycqa.org/en/latest/). These tools ensure a consistent code format
46+
throughout the project. Using pre-commit ensures that linter versions and options are aligned for
47+
all developers. Additionally, there is a CI check in place to enforce that committed code follows
48+
our standards.
49+
50+
To use `pre-commit`, install via `conda` or `pip`:
51+
52+
```bash
53+
conda install -c conda-forge pre-commit
54+
```
55+
56+
```bash
57+
pip install pre-commit
58+
```
59+
60+
Then run pre-commit hooks before committing code:
61+
62+
```bash
63+
pre-commit run
64+
```
65+
66+
Optionally, you may set up the pre-commit hooks to run automatically when you make a git commit. This can be done by running:
67+
68+
```bash
69+
pre-commit install
70+
```
71+
72+
Now code linters and formatters will be run each time you commit changes.
73+
74+
You can skip these checks with `git commit --no-verify` or with the short version `git commit -n`.
75+
76+
4077
### Seasoned developers
4178

4279
Once you have gotten your feet wet and are more comfortable with the code, you

ci/checks/copyright.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
re.compile(r"CMakeLists[.]txt$"),
3636
re.compile(r"CMakeLists_standalone[.]txt$"),
3737
re.compile(r"setup[.]cfg$"),
38-
re.compile(r"[.]flake8[.]cython$"),
3938
re.compile(r"meta[.]yaml$")
4039
]
4140
ExemptFiles = []

ci/checks/style.sh

Lines changed: 6 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -12,69 +12,12 @@ PATH=/opt/conda/bin:$PATH
1212
. /opt/conda/etc/profile.d/conda.sh
1313
conda activate rapids
1414

15-
# Run flake8 and get results/return code
16-
FLAKE=`flake8 --exclude=cpp,thirdparty,__init__.py,versioneer.py && flake8 --config=python/.flake8.cython`
17-
RETVAL=$?
15+
FORMAT_FILE_URL=https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.12/cmake-format-rapids-cmake.json
16+
export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-formats-rapids-cmake.json
17+
mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE})
18+
wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL}
1819

19-
# Output results if failure otherwise show pass
20-
if [ "$FLAKE" != "" ]; then
21-
echo -e "\n\n>>>> FAILED: flake8 style check; begin output\n\n"
22-
echo -e "$FLAKE"
23-
echo -e "\n\n>>>> FAILED: flake8 style check; end output\n\n"
24-
else
25-
echo -e "\n\n>>>> PASSED: flake8 style check\n\n"
26-
fi
27-
28-
# Check for copyright headers in the files modified currently
29-
COPYRIGHT=`python ci/checks/copyright.py --git-modified-only 2>&1`
30-
CR_RETVAL=$?
31-
if [ "$RETVAL" = "0" ]; then
32-
RETVAL=$CR_RETVAL
33-
fi
34-
35-
# Output results if failure otherwise show pass
36-
if [ "$CR_RETVAL" != "0" ]; then
37-
echo -e "\n\n>>>> FAILED: copyright check; begin output\n\n"
38-
echo -e "$COPYRIGHT"
39-
echo -e "\n\n>>>> FAILED: copyright check; end output\n\n"
40-
else
41-
echo -e "\n\n>>>> PASSED: copyright check\n\n"
42-
fi
43-
44-
# Check for a consistent #include syntax
45-
HASH_INCLUDE=`python cpp/scripts/include_checker.py \
46-
cpp/bench \
47-
cpp/include \
48-
cpp/test \
49-
2>&1`
50-
HASH_RETVAL=$?
51-
if [ "$RETVAL" = "0" ]; then
52-
RETVAL=$HASH_RETVAL
53-
fi
54-
55-
# Output results if failure otherwise show pass
56-
if [ "$HASH_RETVAL" != "0" ]; then
57-
echo -e "\n\n>>>> FAILED: #include check; begin output\n\n"
58-
echo -e "$HASH_INCLUDE"
59-
echo -e "\n\n>>>> FAILED: #include check; end output\n\n"
60-
else
61-
echo -e "\n\n>>>> PASSED: #include check\n\n"
62-
fi
63-
64-
# Check for a consistent code format
65-
FORMAT=`python cpp/scripts/run-clang-format.py 2>&1`
66-
FORMAT_RETVAL=$?
67-
if [ "$RETVAL" = "0" ]; then
68-
RETVAL=$FORMAT_RETVAL
69-
fi
70-
71-
# Output results if failure otherwise show pass
72-
if [ "$FORMAT_RETVAL" != "0" ]; then
73-
echo -e "\n\n>>>> FAILED: clang format check; begin output\n\n"
74-
echo -e "$FORMAT"
75-
echo -e "\n\n>>>> FAILED: clang format check; end output\n\n"
76-
else
77-
echo -e "\n\n>>>> PASSED: clang format check\n\n"
78-
fi
20+
# Run pre-commit checks
21+
pre-commit run --hook-stage manual --all-files
7922

8023
exit $RETVAL

0 commit comments

Comments
 (0)