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

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

Open
jameslamb opened this issue Aug 22, 2024 · 0 comments
Open

Comments

@jameslamb
Copy link
Member

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: #766 (comment).

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)
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant