Skip to content

Commit

Permalink
Update linting tool versions and replace isort with ruff (#622)
Browse files Browse the repository at this point in the history
This PR is a small follow-up to #616 (regarding [comments](#616 (review)) by @bdice)

It also introduces a `.git-blame-ignore-revs` to ignore all of the formatting changes introduced in !616 from git blame views

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

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

URL: #622
  • Loading branch information
grlee77 authored Oct 31, 2023
1 parent f95e100 commit a680cf6
Show file tree
Hide file tree
Showing 21 changed files with 60 additions and 82 deletions.
2 changes: 2 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Applied black, isort and ruff globally to the codebase
d489bb3969d67643ad6c3ff62f516b635206ae2a
15 changes: 5 additions & 10 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,20 @@ repos:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: debug-statements
- repo: https://github.com/PyCQA/isort
rev: 5.12.0
hooks:
- id: isort
files: ^python/cucim/src/.*
args: ["--settings-path=python/cucim/pyproject.toml"]
- repo: https://github.com/psf/black
rev: 23.3.0
rev: 23.10.1
hooks:
- id: black
files: (python|legate)/.*
args: ["--config", "python/cucim/pyproject.toml"]
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.0.278
rev: v0.1.3
hooks:
- id: ruff
files: python/.*$
types_or: [python, pyi]
args: [--fix, --exit-non-zero-on-fix]
- repo: https://github.com/codespell-project/codespell
rev: v2.2.4
rev: v2.2.6
hooks:
- id: codespell
args: ["--toml", "python/cucim/pyproject.toml"]
Expand Down
12 changes: 5 additions & 7 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,32 +59,30 @@ The following instructions are for developers and contributors to cuCIM OSS deve
#### Python


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
cuCIM uses [ruff](https://docs.astral.sh/ruff/) and [black](https://black.readthedocs.io/en/stable/) to ensure a consistent code format
throughout the project. `ruff`, and `black` can be installed with
`conda` or `pip`:

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

```bash
pip install isort black ruff
pip install 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
isort --settings-path="python/cucim/pyproject.toml" .
black --config python/cucim/pyproject.toml .
ruff .
ruff --config python/cucim/pyproject.toml .
```

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:
Expand Down
3 changes: 2 additions & 1 deletion benchmarks/skimage/_image_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ def run_benchmark(self, duration=3, verbose=True):
var_kwargs_cpu = self._update_kwargs_arrays(var_kwargs1, "cpu")
var_kwargs_gpu = self._update_kwargs_arrays(var_kwargs1, "gpu")

# Note: brute_force=True on 'gpu' because False is not implemented
# Note: brute_force=True on 'gpu' because False is not
# implemented
if "brute_force" in var_kwargs_gpu:
var_kwargs_gpu["brute_force"] = True

Expand Down
5 changes: 4 additions & 1 deletion benchmarks/skimage/cucim_color_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,10 @@ def main(args):
"-i",
"--img_size",
type=str,
help="Size of input image (omit color channel, it will be appended as needed)",
help=(
"Size of input image (omit color channel, it will be appended "
"as needed)"
),
required=True,
)
parser.add_argument(
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/skimage/cucim_feature_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def main(args):
False,
True,
),
# blob detectors. fixed kwargs here are taken from the docstring examples
# blob detectors, fixed kwargs are taken from the docstring examples
(
"blob_dog",
dict(threshold=0.05, min_sigma=10, max_sigma=40),
Expand Down
2 changes: 0 additions & 2 deletions benchmarks/skimage/cucim_filters_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ def main(args):
# image sizes/shapes
shape = tuple(list(map(int, (args.img_size.split(",")))))

# for shape in [(512, 512), (3840, 2160), (3840, 2160, 3), (192, 192, 192)]:

if function_name in ["gaussian", "unsharp_mask"]:
fixed_kwargs["channel_axis"] = -1 if shape[-1] == 3 else None

Expand Down
2 changes: 1 addition & 1 deletion benchmarks/skimage/cucim_measure_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def main(args):
# _moments.py
("moments", dict(), dict(order=[1, 2, 3, 4]), False, False),
("moments_central", dict(), dict(order=[1, 2, 3]), False, True),
# omitted from benchmarks (only tiny arrays): moments_normalized, moments_hu
# omitted from benchmarks (only tiny arrays): moments_normalized, moments_hu # noqa: E501
("centroid", dict(), dict(), False, True),
("inertia_tensor", dict(), dict(), False, True),
("inertia_tensor_eigvals", dict(), dict(), False, True),
Expand Down
5 changes: 4 additions & 1 deletion benchmarks/skimage/cucim_morphology_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,10 @@ def main(args):
"-i",
"--img_size",
type=str,
help="Size of input image (omit color channel, it will be appended as needed)",
help=(
"Size of input image (omit color channel, it will be appended "
"as needed)"
),
required=True,
)
parser.add_argument(
Expand Down
5 changes: 4 additions & 1 deletion benchmarks/skimage/cucim_registration_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,10 @@ def main(args):
"-i",
"--img_size",
type=str,
help="Size of input image (omit color channel, it will be appended as needed)",
help=(
"Size of input image (omit color channel, it will be appended "
"as needed)"
),
required=True,
)
parser.add_argument(
Expand Down
5 changes: 4 additions & 1 deletion benchmarks/skimage/cucim_restoration_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,10 @@ def main(args):
"-i",
"--img_size",
type=str,
help="Size of input image (omit color channel, it will be appended as needed)",
help=(
"Size of input image (omit color channel, it will be appended "
"as needed)"
),
required=True,
)
parser.add_argument(
Expand Down
5 changes: 4 additions & 1 deletion benchmarks/skimage/cucim_segmentation_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,10 @@ def main(args):
"-i",
"--img_size",
type=str,
help="Size of input image (omit color channel, it will be appended as needed)",
help=(
"Size of input image (omit color channel, it will be appended "
"as needed)"
),
required=True,
)
parser.add_argument(
Expand Down
5 changes: 4 additions & 1 deletion benchmarks/skimage/cucim_transform_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,10 @@ def main(args):
"-i",
"--img_size",
type=str,
help="Size of input image (omit color channel, it will be appended as needed)",
help=(
"Size of input image (omit color channel, it will be appended "
"as needed)"
),
required=True,
)
parser.add_argument(
Expand Down
3 changes: 1 addition & 2 deletions benchmarks/skimage/cupyx_scipy_ndimage_morphology_bench.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import os
import pickle

import cupy
import cupy as cp
import cupyx.scipy.ndimage
import numpy as np
Expand Down Expand Up @@ -143,7 +142,7 @@ def set_args(self, dtype):
structure=structure,
mask=None,
index_str=index_str,
# Note: Benchmark runner will change brute_force to True for the GPU
# Note: Benchmark runner will change brute_force to True for the GPU # noqa: E501
fixed_kwargs=dict(output=None),
var_kwargs=var_kwargs,
)
Expand Down
8 changes: 4 additions & 4 deletions examples/python/gds_whole_slide/lz4_nvcomp.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ def ensure_contiguous_ndarray(buf, max_buffer_size=None, flatten=True):
Notes
-----
This function will not create a copy under any circumstances, it is guaranteed to
return a view on memory exported by `buf`.
This function will not create a copy under any circumstances, it is
guaranteed to return a view on memory exported by `buf`.
"""
arr = ensure_ndarray(buf)

# check for object arrays, these are just memory pointers, actual memory holding
# item data is scattered elsewhere
# check for object arrays, these are just memory pointers, actual memory
# holding item data is scattered elsewhere
if arr.dtype == object:
raise TypeError("object arrays are not supported")

Expand Down
2 changes: 1 addition & 1 deletion experiments/Supporting_Aperio_SVS_Format/benchmark.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def experiment_thread(
print(f" hit: {cache.hit_count} miss: {cache.miss_count}")
print(" ", psutil.virtual_memory())

output_text = f"{datetime.now().strftime('%Y-%m-%d %H:%M:%S')},thread,{cache_strategy},{input_file},{start_location},{patch_size},{num_workers},{openslide_time},{cucim_time},{rasterio_time},{openslide_time / cucim_time},{rasterio_time / cucim_time},{cache_size},{cache.hit_count},{cache.miss_count}\n" # noqa: E501
output_text = f"{datetime.now().strftime('%Y-%m-%d %H:%M:%S')},thread,{cache_strategy},{input_file},{start_location},{patch_size},{num_workers},{openslide_time},{cucim_time},{rasterio_time},{openslide_time / cucim_time},{rasterio_time / cucim_time},{cache_size},{cache.hit_count},{cache.miss_count}\n" # noqa: E501
with open("experiment.txt", "a+") as f:
f.write(output_text)
print(output_text)
Expand Down
53 changes: 10 additions & 43 deletions python/cucim/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -107,46 +107,6 @@ where = ["src"]
[tool.setuptools.package-data]
mypkg = ["*.pyi", "*.h", "*.cu"]

[tool.isort]
profile = "black"
force_single_line = false
line_length = 80
forced_separate = "test_cucim"
force_grid_wrap = 0
multi_line_output = 3
order_by_type = true
combine_as_imports = true
include_trailing_comma = true
known_first_party = [
"cucim",
]
default_section = "THIRDPARTY"
sections = [
"FUTURE",
"STDLIB",
"THIRDPARTY",
"FIRSTPARTY",
"LOCALFOLDER",
]
skip = [
"3rdparty",
"thirdparty",
".eggs",
".git",
".hg",
".mypy_cache",
".tox",
".venv",
"ci",
"cpp",
"_build",
"build",
"build-debug",
"build-release",
"_deps",
"dist",
]

[tool.pytest.ini_options]
# If a pytest section is found in one of the possible config files
# (pytest.ini, tox.ini or setup.cfg), then pytest will not look for any others,
Expand All @@ -173,7 +133,7 @@ testpaths = [

[tool.ruff]
# see: https://docs.astral.sh/ruff/rules/
select = ["E", "F", "W"]
select = ["E", "F", "W", "I"]
fixable = ["ALL"]
exclude = [
# TODO: Remove this in a follow-up where we fix __all__.
Expand All @@ -189,10 +149,17 @@ exclude = [
"__init__.py",
]
line-length = 80
fix = true

[tool.ruff.per-file-ignores]
# "src/cucim/skimage/util/tests/test_shape.py" = ["E201", "E202"]

[tool.ruff.isort]
combine-as-imports = true
force-single-line = false
known-first-party = ["cucim"]
order-by-type = true

[tool.black]
line-length = 80
target-version = ["py39"]
Expand Down Expand Up @@ -225,9 +192,9 @@ exclude = '''
# this is only to allow you to run codespell interactively
# e.g. via
# codespell --toml python/cucim/pyproject.toml . -i 3 -w
skip = "build*,dist,.cache,html,_build,_deps,3rdparty,_static,generated,latex,.git,*.ipynb,test_data/input/LICENSE-3rdparty,jitify_testing"
skip = "build*,dist,.cache,html,_build,_deps,3rdparty/*,_static,generated,latex,.git,*.ipynb,test_data/input/LICENSE-3rdparty,jitify_testing"
# ignore-regex = ""
ignore-words-list = "ans,coo,boun,bui,gool,hart,lond,nd,paeth,unser,wronly"
ignore-words-list = "ans,coo,boun,bui,gool,hart,lond,manuel,nd,paeth,unser,wronly"
quiet-level = 3

# to undo: ./test_data/input/LICENSE-3rdparty
2 changes: 1 addition & 1 deletion python/cucim/src/cucim/skimage/segmentation/_chan_vese.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ def _cv_small_disk(image_size):

def _cv_init_level_set(init_level_set, image_shape, dtype=cp.float64):
"""Generates an initial level set function conditional on input arguments.""" # noqa: E501
if type(init_level_set) == str:
if isinstance(init_level_set, str):
if init_level_set == "checkerboard":
res = _cv_checkerboard(image_shape, 5, dtype)
elif init_level_set == "disk":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# to avoid bias, but as we are relying on the index map returned
# by the scipy.ndimage distance transform, what actually happens
# is determined by the upstream implementation of the distance
# tansform, thus we don't give any guarantees for the edge case pixels.
# transform, thus we don't give any guarantees for the edge case pixels.
#
# Regardless, it seems prudent to have a test including an edge case
# so we can detect whether future upstream changes in scipy.ndimage
Expand Down
2 changes: 1 addition & 1 deletion python/cucim/src/cucim/skimage/transform/_warps.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ def warp_coords(coord_map, shape, dtype=cp.float64):
images used by `warp()`.
It is provided separately from `warp` to give additional flexibility to
users who would like, for example, to re-use a particular coordinate
users who would like, for example, to reuse a particular coordinate
mapping, to use specific dtypes at various points along the the
image-warping process, or to implement different post-processing logic
than `warp` performs after the call to `ndi.map_coordinates`.
Expand Down
2 changes: 1 addition & 1 deletion scripts/auditwheel_repair.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def inwheelctx_enter(self):
# with open(wheel_path, 'r') as f:
# wheel_text = f.read()

# wheel_text = wheel_text.replace('Root-Is-Purelib: true', 'Root-Is-Purelib: false')
# wheel_text = wheel_text.replace('Root-Is-Purelib: true', 'Root-Is-Purelib: false') # noqa: E501

# with open(wheel_path, 'w') as f:
# f.write(wheel_text)
Expand Down

0 comments on commit a680cf6

Please sign in to comment.