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.

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

Run linters using pre-commit #965

Merged
merged 11 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
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:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll mention it here because I haven't fixed it yet in cuDF and this is a new file: the indentation of this file is ...aggressive. I'd prefer to see something more like 2 or 4 spaces instead of 8 spaces in some places and 2 or 4 in others. Maybe there's a standard YAML formatter that can be run once (no need to enforce the format with tooling, just get it into a normalized state).

Copy link
Contributor

Choose a reason for hiding this comment

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

I usually add https://github.com/jumanjihouse/pre-commit-hook-yamlfmt to the pre-commit hooks for this reason.

- repo: https://github.com/PyCQA/isort
cjnolet marked this conversation as resolved.
Show resolved Hide resolved
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"
cjnolet marked this conversation as resolved.
Show resolved Hide resolved
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