rapidsai / cucim

cuCIM - RAPIDS GPU-accelerated image processing library
https://docs.rapids.ai/api/cucim/stable/
Apache License 2.0
356 stars 61 forks source link

investigate E721 (pycodestyle) ruff error about == comparisons of types #767

Open jameslamb opened 3 months ago

jameslamb commented 3 months ago

Description

The version of ruff used in this repo was upgraded to v0.6.1 in #766.

That raised a new class of error, like this:

python/cucim/src/cucim/skimage/morphology/tests/test_misc.py:52:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
   |
50 |     expected_out = cp.empty_like(test_image, dtype=out_dtype)
51 | 
52 |     if out_dtype != bool:
   |        ^^^^^^^^^^^^^^^^^ E721
53 |         # object with only 1 label will warn on non-bool output dtype
54 |         exp_warn = ["Only one label was provided"]

As @jakirkham noted there, it might not be safe to use is / is not when something like a numpy.dtype is on either side of the comparison: https://github.com/rapidsai/cucim/pull/766#issuecomment-2305380224.

In that PR, we simply ignored that ruff rule.

This issue documents the work of investigating each of these cases and deciding whether to accept any of ruff's suggestions.

Approach

Remove E721 from the list of ignored rules in the [ruff.lint] table in python/cucim/pyproject.toml.

Run the linter.

pre-commit run --all-files

Investigate any errors raised.

When this was written, 15 such things were found across cucim:

full list (click me) ```text python/cucim/src/cucim/skimage/_shared/tests/test_utils.py:94:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 92 | assert ( 93 | _validate_interpolation_order(dtype, None) == 0 94 | if dtype == bool | ^^^^^^^^^^^^^ E721 95 | else 1 96 | ) | python/cucim/src/cucim/skimage/_shared/tests/test_utils.py:101:10: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 99 | with pytest.raises(ValueError): 100 | _validate_interpolation_order(dtype, order) 101 | elif dtype == bool and order != 0: | ^^^^^^^^^^^^^ E721 102 | # Deprecated order for bool array 103 | with pytest.raises(ValueError): | python/cucim/src/cucim/skimage/_shared/utils.py:812:21: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 811 | if order is None: 812 | return 0 if image_dtype == bool else 1 | ^^^^^^^^^^^^^^^^^^^ E721 813 | 814 | if order < 0 or order > 5: | python/cucim/src/cucim/skimage/_shared/utils.py:819:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 817 | ) 818 | 819 | if image_dtype == bool and order != 0: | ^^^^^^^^^^^^^^^^^^^ E721 820 | raise ValueError( 821 | "Input image dtype is bool. Interpolation is not defined " | python/cucim/src/cucim/skimage/color/colorlabel.py:252:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 251 | new_type = np.min_scalar_type(int(label.max())) 252 | if new_type == bool: | ^^^^^^^^^^^^^^^^ E721 253 | new_type = np.uint8 254 | label = label.astype(new_type) | python/cucim/src/cucim/skimage/exposure/exposure.py:502:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 500 | # pair of values: always return float. 501 | return utils._supported_float_type(image_dtype) 502 | if type(dtype_or_range) == type: | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721 503 | # already a type: return it 504 | return dtype_or_range | python/cucim/src/cucim/skimage/measure/_regionprops.py:141:63: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 139 | } 140 | 141 | OBJECT_COLUMNS = [col for col, dtype in COL_DTYPES.items() if dtype == object] | ^^^^^^^^^^^^^^^ E721 142 | 143 | PROP_VALS = set(PROPS.values()) | python/cucim/src/cucim/skimage/measure/tests/test_regionprops.py:1226:20: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 1225 | if col in OBJECT_COLUMNS: 1226 | assert COL_DTYPES[col] == object | ^^^^^^^^^^^^^^^^^^^^^^^^^ E721 1227 | continue | python/cucim/src/cucim/skimage/measure/tests/test_regionprops.py:1244:17: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 1242 | if cp.issubdtype(t, cp.floating): 1243 | assert ( 1244 | COL_DTYPES[col] == float | ^^^^^^^^^^^^^^^^^^^^^^^^ E721 1245 | ), f"{col} dtype {t} {msg} {COL_DTYPES[col]}" 1246 | elif cp.issubdtype(t, cp.integer): | python/cucim/src/cucim/skimage/measure/tests/test_regionprops.py:1248:17: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 1246 | elif cp.issubdtype(t, cp.integer): 1247 | assert ( 1248 | COL_DTYPES[col] == int | ^^^^^^^^^^^^^^^^^^^^^^ E721 1249 | ), f"{col} dtype {t} {msg} {COL_DTYPES[col]}" 1250 | else: | python/cucim/src/cucim/skimage/morphology/tests/test_misc.py:52:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 50 | expected_out = cp.empty_like(test_image, dtype=out_dtype) 51 | 52 | if out_dtype != bool: | ^^^^^^^^^^^^^^^^^ E721 53 | # object with only 1 label will warn on non-bool output dtype 54 | exp_warn = ["Only one label was provided"] | python/cucim/src/cucim/skimage/segmentation/_chan_vese.py:417:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 415 | float_dtype = _supported_float_type(image.dtype) 416 | phi = _cv_init_level_set(init_level_set, image.shape, dtype=float_dtype) 417 | if type(phi) != cp.ndarray or phi.shape != image.shape: | ^^^^^^^^^^^^^^^^^^^^^^^ E721 418 | raise ValueError( 419 | "The dimensions of initial level set do not " | python/cucim/src/cucim/skimage/transform/_geometric.py:916:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 914 | # combination of the same types result in a transformation of this 915 | # type again, otherwise use general projective transformation 916 | if type(self) == type(other): | ^^^^^^^^^^^^^^^^^^^^^^^^^ E721 917 | tform = self.__class__ 918 | else: | python/cucim/src/cucim/skimage/transform/_warps.py:173:17: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 171 | if anti_aliasing is None: 172 | anti_aliasing = ( 173 | not input_type == bool | ^^^^^^^^^^^^^^^^^^ E721 174 | and not (cp.issubdtype(input_type, cp.integer) and order == 0) 175 | and any(x < y for x, y in zip(output_shape, input_shape)) | python/cucim/src/cucim/skimage/transform/_warps.py:178:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks | 176 | ) 177 | 178 | if input_type == bool and anti_aliasing: | ^^^^^^^^^^^^^^^^^^ E721 179 | raise ValueError("anti_aliasing must be False for boolean images") | Found 15 errors. ```