Skip to content

Commit

Permalink
update linters and move their configurations from setup.cfg to pyproj…
Browse files Browse the repository at this point in the history
…ect.toml (#616)

The first commit here is from #615

This MR makes multiple linting-related changes to cuCIM
- moves existing `isort` config from `setup.cfg` to a new `pyproject.toml` file
- update `isort` conventions to match other RAPIDS projects
- add `black` and apply it (use `fmt::off`/`fmt::on` as needed to preserve formatting of look-up tables and test cases)
- replace use of `flake8` with `ruff`
- add config for `codespell` and apply recommended fixes across both C++ and Python code
- update CONTRIBUTING.md instructions to reflect these changes
- Pre-commit hooks for `black`, `ruff` and `codespell` were added. The pre-commit hook for `flake8` was removed.

The only non-linting change is
- move pytest configuration from `setup.cfg` to `pyproject.toml`

I plan to leave any build-system related changes to `pyproject.toml` to a follow-up MR. This MR focuses only on linting/testing.

It will probably be easiest to review the individual commits. Some, like applying `black` are quite large.

Fixes #23

Authors:
  - Gregory Lee (https://github.com/grlee77)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Ray Douglass (https://github.com/raydouglass)
  - Gigon Bae (https://github.com/gigony)
  - https://github.com/jakirkham

URL: #616
  • Loading branch information
grlee77 authored Oct 27, 2023
1 parent 2492a48 commit d489bb3
Show file tree
Hide file tree
Showing 278 changed files with 13,311 additions and 7,886 deletions.
25 changes: 19 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,23 @@ repos:
hooks:
- id: isort
files: ^python/cucim/src/.*
args: ["--settings-path=python/cucim/setup.cfg"]
- repo: https://github.com/PyCQA/flake8
rev: 6.0.0
args: ["--settings-path=python/cucim/pyproject_.toml"]
- repo: https://github.com/psf/black
rev: 23.3.0
hooks:
- id: flake8
args: ["--config=python/cucim/setup.cfg"]
files: ^python/cucim/.*
- id: black
files: (python|legate)/.*
args: ["--config", "python/cucim/pyproject_.toml"]
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.0.278
hooks:
- id: ruff
files: python/.*$
args: ["--exclude", "__init__.py"]
- repo: https://github.com/codespell-project/codespell
rev: v2.2.4
hooks:
- id: codespell
args: ["--toml", "python/cucim/pyproject_.toml"]
additional_dependencies:
- tomli
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
## 🐛 Bug Fixes

- Fix bug in median filter with non-uniform footprint ([#521](https://github.com/rapidsai/cucim/pull/521)) [@grlee77](https://github.com/grlee77)
- use cp.around instead of cp.round for CuPy 10.x compatiblity ([#508](https://github.com/rapidsai/cucim/pull/508)) [@grlee77](https://github.com/grlee77)
- use cp.around instead of cp.round for CuPy 10.x compatibility ([#508](https://github.com/rapidsai/cucim/pull/508)) [@grlee77](https://github.com/grlee77)
- Fix error in LZ4-compressed Zarr writing demo ([#506](https://github.com/rapidsai/cucim/pull/506)) [@grlee77](https://github.com/grlee77)
- Normalize whitespace. ([#474](https://github.com/rapidsai/cucim/pull/474)) [@bdice](https://github.com/bdice)

Expand Down
32 changes: 24 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,43 @@ The following instructions are for developers and contributors to cuCIM OSS deve

#### Python

cuCIM uses [isort](https://readthedocs.org/projects/isort/), and
[flake8](http://flake8.pycqa.org/en/latest/) to ensure a consistent code format

cuCIM uses [isort](https://readthedocs.org/projects/isort/), [ruff](https://docs.astral.sh/ruff/) and [black](https://black.readthedocs.io/en/stable/) to ensure a consistent code format
throughout the project. `isort`, and `flake8` can be installed with
`conda` or `pip`:

```bash
conda install isort flake8
conda install isort black ruff
```

```bash
pip install isort black ruff
```

These tools are used to auto-format the Python code in the repository. Additionally, there is a CI check in place to enforce that the committed code follows our standards. To run only for the python/cucim folder, change to that folder and run

```bash
isort .
black .
ruff .
```

To also check formatting in top-level folders like `benchmarks`, `examples` and `experiments`, these tools can also be run from the top level of the repository as follows:

```bash
pip install isort flake8
isort --settings-path="python/cucim/pyproject.toml" .
black --config python/cucim/pyproject.toml .
ruff .
```

These tools are used to auto-format the Python code in the repository. Additionally, there is a CI check in place to enforce
that the committed code follows our standards. You can use the tools to
automatically format your python code by running:
In addition to these tools, [codespell](https://github.com/codespell-project/codespell) can be used to help diagnose and interactively fix spelling errors in both Python and C++ code. It can also be run from the top level of the repository in interactive mode using:

```bash
isort --atomic python/**/*.py
codespell --toml python/cucim/pyproject.toml . -i 3 -w
```

If codespell is finding false positives in newly added code, the `ignore-words-list` entry of the `tool.codespell` section in `pyproject.toml` can be updated as needed.

### Get libcucim Dependencies

Compiler requirements:
Expand Down
41 changes: 27 additions & 14 deletions benchmarks/skimage/_image_bench.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import itertools
import math
import re
import subprocess
import time
import types
from collections import abc
import re
import subprocess

import cupy as cp
import cupyx.scipy.ndimage
import numpy as np
import pandas as pd
import scipy.ndimage
import skimage.data

from cucim.time import repeat


Expand All @@ -32,13 +33,11 @@ def __init__(
fixed_kwargs={},
var_kwargs={},
index_str=None, # extra string to append to dataframe index
# set_args_kwargs={}, # for passing additional arguments to custom set_args method
module_cpu=scipy.ndimage,
module_gpu=cupyx.scipy.ndimage,
function_is_generator=False,
run_cpu=True
run_cpu=True,
):

self.shape = shape
self.function_name = function_name
self.fixed_kwargs_cpu = self._update_kwargs_arrays(fixed_kwargs, "cpu")
Expand Down Expand Up @@ -171,17 +170,23 @@ def run_benchmark(self, duration=3, verbose=True):
self.func_gpu, self.args_gpu, kw_gpu, duration, cpu=False
)
print("Number of Repetitions : ", rep_kwargs_gpu)
perf_gpu = repeat(self.func_gpu, self.args_gpu, kw_gpu, **rep_kwargs_gpu)
perf_gpu = repeat(
self.func_gpu, self.args_gpu, kw_gpu, **rep_kwargs_gpu
)

df.at[index, "shape"] = f"{self.shape}"
# df.at[index, "description"] = index
df.at[index, "function_name"] = self.function_name
df.at[index, "dtype"] = np.dtype(dtype).name
df.at[index, "ndim"] = len(self.shape)

if self.run_cpu == True:
perf = repeat(self.func_cpu, self.args_cpu, kw_cpu, **rep_kwargs_cpu)
df.at[index, "GPU accel"] = perf.cpu_times.mean() / perf_gpu.gpu_times.mean()
if self.run_cpu is True:
perf = repeat(
self.func_cpu, self.args_cpu, kw_cpu, **rep_kwargs_cpu
)
df.at[index, "GPU accel"] = (
perf.cpu_times.mean() / perf_gpu.gpu_times.mean()
)
df.at[index, "CPU: host (mean)"] = perf.cpu_times.mean()
df.at[index, "CPU: host (std)"] = perf.cpu_times.std()

Expand All @@ -191,14 +196,22 @@ def run_benchmark(self, duration=3, verbose=True):
df.at[index, "GPU: device (std)"] = perf_gpu.gpu_times.std()
with cp.cuda.Device() as device:
props = cp.cuda.runtime.getDeviceProperties(device.id)
gpu_name = props['name'].decode()
gpu_name = props["name"].decode()

df.at[index, "GPU: DEV Name"] = [gpu_name for i in range(len(df))]
df.at[index, "GPU: DEV Name"] = [
gpu_name for i in range(len(df))
]
cmd = "cat /proc/cpuinfo"
cpuinfo = subprocess.check_output(cmd, shell=True).strip()
cpu_name = re.search("\nmodel name.*\n", cpuinfo.decode()).group(0).strip('\n')
cpu_name = cpu_name.replace('model name\t: ', '')
df.at[index, "CPU: DEV Name"] = [cpu_name for i in range(len(df))]
cpu_name = (
re.search("\nmodel name.*\n", cpuinfo.decode())
.group(0)
.strip("\n")
)
cpu_name = cpu_name.replace("model name\t: ", "")
df.at[index, "CPU: DEV Name"] = [
cpu_name for i in range(len(df))
]

# accelerations[arr_index] = df.at[index, "GPU accel"]
if verbose:
Expand Down
Loading

0 comments on commit d489bb3

Please sign in to comment.