From a680cf6a2a6684d8cee9c3521991623c0a3b0d2d Mon Sep 17 00:00:00 2001 From: Gregory Lee Date: Tue, 31 Oct 2023 11:44:48 -0700 Subject: [PATCH] Update linting tool versions and replace isort with ruff (#622) This PR is a small follow-up to #616 (regarding [comments](https://github.com/rapidsai/cucim/pull/616#pullrequestreview-1698138844) 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: https://github.com/rapidsai/cucim/pull/622 --- .git-blame-ignore-revs | 2 + .pre-commit-config.yaml | 15 ++---- CONTRIBUTING.md | 12 ++--- benchmarks/skimage/_image_bench.py | 3 +- benchmarks/skimage/cucim_color_bench.py | 5 +- benchmarks/skimage/cucim_feature_bench.py | 2 +- benchmarks/skimage/cucim_filters_bench.py | 2 - benchmarks/skimage/cucim_measure_bench.py | 2 +- benchmarks/skimage/cucim_morphology_bench.py | 5 +- .../skimage/cucim_registration_bench.py | 5 +- benchmarks/skimage/cucim_restoration_bench.py | 5 +- .../skimage/cucim_segmentation_bench.py | 5 +- benchmarks/skimage/cucim_transform_bench.py | 5 +- .../cupyx_scipy_ndimage_morphology_bench.py | 3 +- examples/python/gds_whole_slide/lz4_nvcomp.py | 8 +-- .../Supporting_Aperio_SVS_Format/benchmark.py | 2 +- python/cucim/pyproject.toml | 53 ++++--------------- .../cucim/skimage/segmentation/_chan_vese.py | 2 +- .../segmentation/tests/test_expand_labels.py | 2 +- .../src/cucim/skimage/transform/_warps.py | 2 +- scripts/auditwheel_repair.py | 2 +- 21 files changed, 60 insertions(+), 82 deletions(-) create mode 100644 .git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 000000000..5b8d90825 --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,2 @@ +# Applied black, isort and ruff globally to the codebase +d489bb3969d67643ad6c3ff62f516b635206ae2a diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4413211c8..0290c0e8f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -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"] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0bb7757fe..16c29fcb6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,22 +59,21 @@ 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 . ``` @@ -82,9 +81,8 @@ 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: diff --git a/benchmarks/skimage/_image_bench.py b/benchmarks/skimage/_image_bench.py index 571588fee..9d55c6be0 100644 --- a/benchmarks/skimage/_image_bench.py +++ b/benchmarks/skimage/_image_bench.py @@ -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 diff --git a/benchmarks/skimage/cucim_color_bench.py b/benchmarks/skimage/cucim_color_bench.py index 3e57e1afb..b9886d2b8 100644 --- a/benchmarks/skimage/cucim_color_bench.py +++ b/benchmarks/skimage/cucim_color_bench.py @@ -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( diff --git a/benchmarks/skimage/cucim_feature_bench.py b/benchmarks/skimage/cucim_feature_bench.py index d767fc6e0..b390f02e6 100644 --- a/benchmarks/skimage/cucim_feature_bench.py +++ b/benchmarks/skimage/cucim_feature_bench.py @@ -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), diff --git a/benchmarks/skimage/cucim_filters_bench.py b/benchmarks/skimage/cucim_filters_bench.py index a6771f59b..9dcbf2867 100644 --- a/benchmarks/skimage/cucim_filters_bench.py +++ b/benchmarks/skimage/cucim_filters_bench.py @@ -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 diff --git a/benchmarks/skimage/cucim_measure_bench.py b/benchmarks/skimage/cucim_measure_bench.py index ef99a74d0..15ec470ce 100644 --- a/benchmarks/skimage/cucim_measure_bench.py +++ b/benchmarks/skimage/cucim_measure_bench.py @@ -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), diff --git a/benchmarks/skimage/cucim_morphology_bench.py b/benchmarks/skimage/cucim_morphology_bench.py index a4877af8c..47658e502 100644 --- a/benchmarks/skimage/cucim_morphology_bench.py +++ b/benchmarks/skimage/cucim_morphology_bench.py @@ -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( diff --git a/benchmarks/skimage/cucim_registration_bench.py b/benchmarks/skimage/cucim_registration_bench.py index 5f9802bd1..86070ecb5 100644 --- a/benchmarks/skimage/cucim_registration_bench.py +++ b/benchmarks/skimage/cucim_registration_bench.py @@ -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( diff --git a/benchmarks/skimage/cucim_restoration_bench.py b/benchmarks/skimage/cucim_restoration_bench.py index 4e3711519..064b9a068 100644 --- a/benchmarks/skimage/cucim_restoration_bench.py +++ b/benchmarks/skimage/cucim_restoration_bench.py @@ -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( diff --git a/benchmarks/skimage/cucim_segmentation_bench.py b/benchmarks/skimage/cucim_segmentation_bench.py index 38add9d64..1efe2dd9f 100644 --- a/benchmarks/skimage/cucim_segmentation_bench.py +++ b/benchmarks/skimage/cucim_segmentation_bench.py @@ -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( diff --git a/benchmarks/skimage/cucim_transform_bench.py b/benchmarks/skimage/cucim_transform_bench.py index 74a9b4d0c..3d64d60d3 100644 --- a/benchmarks/skimage/cucim_transform_bench.py +++ b/benchmarks/skimage/cucim_transform_bench.py @@ -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( diff --git a/benchmarks/skimage/cupyx_scipy_ndimage_morphology_bench.py b/benchmarks/skimage/cupyx_scipy_ndimage_morphology_bench.py index d83ee72f9..a177c99f9 100644 --- a/benchmarks/skimage/cupyx_scipy_ndimage_morphology_bench.py +++ b/benchmarks/skimage/cupyx_scipy_ndimage_morphology_bench.py @@ -1,7 +1,6 @@ import os import pickle -import cupy import cupy as cp import cupyx.scipy.ndimage import numpy as np @@ -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, ) diff --git a/examples/python/gds_whole_slide/lz4_nvcomp.py b/examples/python/gds_whole_slide/lz4_nvcomp.py index fb94ed535..19fc5f3d9 100644 --- a/examples/python/gds_whole_slide/lz4_nvcomp.py +++ b/examples/python/gds_whole_slide/lz4_nvcomp.py @@ -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") diff --git a/experiments/Supporting_Aperio_SVS_Format/benchmark.py b/experiments/Supporting_Aperio_SVS_Format/benchmark.py index ec65920c2..17085a8e9 100644 --- a/experiments/Supporting_Aperio_SVS_Format/benchmark.py +++ b/experiments/Supporting_Aperio_SVS_Format/benchmark.py @@ -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) diff --git a/python/cucim/pyproject.toml b/python/cucim/pyproject.toml index 2b3143ff7..8af8dca84 100644 --- a/python/cucim/pyproject.toml +++ b/python/cucim/pyproject.toml @@ -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, @@ -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__. @@ -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"] @@ -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 diff --git a/python/cucim/src/cucim/skimage/segmentation/_chan_vese.py b/python/cucim/src/cucim/skimage/segmentation/_chan_vese.py index 2b7df17af..81c393794 100644 --- a/python/cucim/src/cucim/skimage/segmentation/_chan_vese.py +++ b/python/cucim/src/cucim/skimage/segmentation/_chan_vese.py @@ -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": diff --git a/python/cucim/src/cucim/skimage/segmentation/tests/test_expand_labels.py b/python/cucim/src/cucim/skimage/segmentation/tests/test_expand_labels.py index d839ecd88..4955db8c0 100644 --- a/python/cucim/src/cucim/skimage/segmentation/tests/test_expand_labels.py +++ b/python/cucim/src/cucim/skimage/segmentation/tests/test_expand_labels.py @@ -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 diff --git a/python/cucim/src/cucim/skimage/transform/_warps.py b/python/cucim/src/cucim/skimage/transform/_warps.py index 6ba5a85a6..88f6f0346 100644 --- a/python/cucim/src/cucim/skimage/transform/_warps.py +++ b/python/cucim/src/cucim/skimage/transform/_warps.py @@ -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`. diff --git a/scripts/auditwheel_repair.py b/scripts/auditwheel_repair.py index c2461dc63..2581ea977 100644 --- a/scripts/auditwheel_repair.py +++ b/scripts/auditwheel_repair.py @@ -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)