Skip to content

Commit

Permalink
Run linters using pre-commit (#965)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
benfred authored Nov 10, 2022
1 parent 7176d94 commit 560c66b
Show file tree
Hide file tree
Showing 62 changed files with 1,835 additions and 1,372 deletions.
98 changes: 98 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# Copyright (c) 2022, NVIDIA CORPORATION.

repos:
- repo: https://github.com/PyCQA/isort
rev: 5.10.1
hooks:
- id: isort
# Use the config file specific to each subproject so that each
# project can specify its own first/third-party packages.
args: ["--config-root=python/", "--resolve-all-configs"]
files: python/.*
types_or: [python, cython, pyi]
- repo: https://github.com/psf/black
rev: 22.3.0
hooks:
- id: black
files: python/.*
# Explicitly specify the pyproject.toml at the repo root, not per-project.
args: ["--config", "pyproject.toml"]
exclude: .*_version.py,.*versioneer.py
- repo: https://github.com/PyCQA/flake8
rev: 5.0.4
hooks:
- id: flake8
args: ["--config=setup.cfg"]
files: python/.*$
types: [file]
types_or: [python, cython]
additional_dependencies: ["flake8-force"]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: 'v0.971'
hooks:
- id: mypy
additional_dependencies: [types-cachetools]
args: ["--config-file=setup.cfg",
"python/pylibraft/pylibraft",
"python/raft-dask/raft_dask"]
pass_filenames: false
exclude: .*_version.py
- repo: https://github.com/PyCQA/pydocstyle
rev: 6.1.1
hooks:
- id: pydocstyle
args: ["--config=setup.cfg"]
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v11.1.0
hooks:
- id: clang-format
types_or: [c, c++, cuda]
args: ["-fallback-style=none", "-style=file", "-i"]
exclude: cpp/include/raft/thirdparty/.*
- repo: local
hooks:
- id: no-deprecationwarning
name: no-deprecationwarning
description: 'Enforce that DeprecationWarning is not introduced (use FutureWarning instead)'
entry: '(category=|\s)DeprecationWarning[,)]'
language: pygrep
types_or: [python, cython]
- id: cmake-format
name: cmake-format
entry: ./cpp/scripts/run-cmake-format.sh cmake-format
language: python
types: [cmake]
exclude: .*/thirdparty/.*
# Note that pre-commit autoupdate does not update the versions
# of dependencies, so we'll have to update this manually.
additional_dependencies:
- cmakelang==0.6.13
verbose: true
require_serial: true
- id: cmake-lint
name: cmake-lint
entry: ./cpp/scripts/run-cmake-format.sh cmake-lint
language: python
types: [cmake]
# Note that pre-commit autoupdate does not update the versions
# of dependencies, so we'll have to update this manually.
additional_dependencies:
- cmakelang==0.6.13
verbose: true
require_serial: true
exclude: .*/thirdparty/.*
- id: copyright-check
name: copyright-check
entry: python ./ci/checks/copyright.py --git-modified-only --update-current-year
language: python
pass_filenames: false
additional_dependencies: [gitpython]
- id: include-check
name: include-check
entry: python ./cpp/scripts/include_checker.py cpp/bench cpp/include cpp/test
pass_filenames: false
language: python
additional_dependencies: [gitpython]

default_language_version:
python: python3
37 changes: 37 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,43 @@ into three categories:
Remember, if you are unsure about anything, don't hesitate to comment on issues
and ask for clarifications!


### Python / Pre-commit hooks

RAFT uses [pre-commit](https://pre-commit.com/) to execute code linters and formatters such as
[Black](https://black.readthedocs.io/en/stable/), [isort](https://pycqa.github.io/isort/), and
[flake8](https://flake8.pycqa.org/en/latest/). These tools ensure a consistent code format
throughout the project. Using pre-commit ensures that linter versions and options are aligned for
all developers. Additionally, there is a CI check in place to enforce that committed code follows
our standards.

To use `pre-commit`, install via `conda` or `pip`:

```bash
conda install -c conda-forge pre-commit
```

```bash
pip install pre-commit
```

Then run pre-commit hooks before committing code:

```bash
pre-commit run
```

Optionally, you may set up the pre-commit hooks to run automatically when you make a git commit. This can be done by running:

```bash
pre-commit install
```

Now code linters and formatters will be run each time you commit changes.

You can skip these checks with `git commit --no-verify` or with the short version `git commit -n`.


### Seasoned developers

Once you have gotten your feet wet and are more comfortable with the code, you
Expand Down
1 change: 0 additions & 1 deletion ci/checks/copyright.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
re.compile(r"CMakeLists[.]txt$"),
re.compile(r"CMakeLists_standalone[.]txt$"),
re.compile(r"setup[.]cfg$"),
re.compile(r"[.]flake8[.]cython$"),
re.compile(r"meta[.]yaml$")
]
ExemptFiles = []
Expand Down
69 changes: 6 additions & 63 deletions ci/checks/style.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,69 +12,12 @@ PATH=/opt/conda/bin:$PATH
. /opt/conda/etc/profile.d/conda.sh
conda activate rapids

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

# Output results if failure otherwise show pass
if [ "$FLAKE" != "" ]; then
echo -e "\n\n>>>> FAILED: flake8 style check; begin output\n\n"
echo -e "$FLAKE"
echo -e "\n\n>>>> FAILED: flake8 style check; end output\n\n"
else
echo -e "\n\n>>>> PASSED: flake8 style check\n\n"
fi

# Check for copyright headers in the files modified currently
COPYRIGHT=`python ci/checks/copyright.py --git-modified-only 2>&1`
CR_RETVAL=$?
if [ "$RETVAL" = "0" ]; then
RETVAL=$CR_RETVAL
fi

# Output results if failure otherwise show pass
if [ "$CR_RETVAL" != "0" ]; then
echo -e "\n\n>>>> FAILED: copyright check; begin output\n\n"
echo -e "$COPYRIGHT"
echo -e "\n\n>>>> FAILED: copyright check; end output\n\n"
else
echo -e "\n\n>>>> PASSED: copyright check\n\n"
fi

# Check for a consistent #include syntax
HASH_INCLUDE=`python cpp/scripts/include_checker.py \
cpp/bench \
cpp/include \
cpp/test \
2>&1`
HASH_RETVAL=$?
if [ "$RETVAL" = "0" ]; then
RETVAL=$HASH_RETVAL
fi

# Output results if failure otherwise show pass
if [ "$HASH_RETVAL" != "0" ]; then
echo -e "\n\n>>>> FAILED: #include check; begin output\n\n"
echo -e "$HASH_INCLUDE"
echo -e "\n\n>>>> FAILED: #include check; end output\n\n"
else
echo -e "\n\n>>>> PASSED: #include check\n\n"
fi

# Check for a consistent code format
FORMAT=`python cpp/scripts/run-clang-format.py 2>&1`
FORMAT_RETVAL=$?
if [ "$RETVAL" = "0" ]; then
RETVAL=$FORMAT_RETVAL
fi

# Output results if failure otherwise show pass
if [ "$FORMAT_RETVAL" != "0" ]; then
echo -e "\n\n>>>> FAILED: clang format check; begin output\n\n"
echo -e "$FORMAT"
echo -e "\n\n>>>> FAILED: clang format check; end output\n\n"
else
echo -e "\n\n>>>> PASSED: clang format check\n\n"
fi
# Run pre-commit checks
pre-commit run --hook-stage manual --all-files

exit $RETVAL
Loading

0 comments on commit 560c66b

Please sign in to comment.