🔨 Improve mypy Workflow and Fix Typing Errors (#1045)

This pull request includes several bug fixes and improvements across utility functions, type checking, and testing in the codebase. The main focus is on improving error handling in image reading, refining type conversions, and enhancing test coverage for edge cases.

**Bug fixes and error handling improvements:**

* Improved `imread` in `utils/misc.py` to raise a `FileNotFoundError` when the image path does not exist and an `OSError` when OpenCV fails to read the image, ensuring clearer error reporting.
* Updated `cast_to_min_dtype` in `utils/misc.py` to use a tuple of (max_value, dtype) pairs for more robust and readable type selection.
* Fixed a bug in `dict_to_store_semantic_segmentor` by converting `layer_list` to a Python list before further processing, ensuring compatibility with downstream code. [[1]](diffhunk://#diff-dfa5a361c3c57e515728d23b53be87e0ee8da01ea04ae9cd6c39770fe3f1aaedR1480-R1487) [[2]](diffhunk://#diff-dfa5a361c3c57e515728d23b53be87e0ee8da01ea04ae9cd6c39770fe3f1aaedL1488-R1496)
* Refined coordinate type casting in `filter_coordinates` in `tools/patchextraction.py` to use `cast` for explicit type annotation.

**Testing improvements:**

* Added tests for `imread` to verify correct exceptions are raised for invalid paths and unreadable images, increasing test coverage for error scenarios.
* Added a test to ensure `compile_model` raises a `ValueError` when called with `None`, improving robustness of model compilation utilities.

**Type checking and code clarity:**

* Added a debug step to the MyPy type checking workflow and changed the import following mode to `silent` for better stability.
* Added missing imports and cleaned up imports in test files for clarity and correctness. [[1]](diffhunk://#diff-33c13e0b177bacd2f02e29bcb8aea5b49e7ce34901fd8f41fefb65defba1bd33R7) [[2]](diffhunk://#diff-33c13e0b177bacd2f02e29bcb8aea5b49e7ce34901fd8f41fefb65defba1bd33R46)
* Minor refactorings for clarity and explicitness, such as assigning the `attention` attribute in model utilities and improving variable naming in `tools/graph.py` and `utils/image.py`. [[1]](diffhunk://#diff-6eae2ccdde40d5d9c4749fb799b7777ceafbf1778f7b7f4b317af2972dde4225R328-R329) [[2]](diffhunk://#diff-a57a552474d66ef5961593e747fd21ba3fe25530b5a694dd5e8e467614bbc816L387-R391) [[3]](diffhunk://#diff-57f834801126808967286c68c43cb74bb25efaab0369210d0184e2576da66f3bL650-R655) [[4]](diffhunk://#diff-7fc02666e28590e37ff11a7ef254c9b2eb52eaf0f08017184d6e614df176ee80L6-R6)

**Model utility improvements:**

* Added an explicit check in `compile_model` to raise a `ValueError` if `model` is `None`, preventing silent failures.

These changes collectively improve code reliability, maintainability, and test coverage.

---------

Co-authored-by: Jiaqi Lv <jiaqilv@Jiaqis-MacBook-Pro.local>
Co-authored-by: Shan E Ahmed Raza <13048456+shaneahmed@users.noreply.github.com>

Author: 3 people

.github/workflows/mypy-type-check.yml

@@ -38,9 +38,15 @@ jobs:
         pip install torch torchvision --index-url https://download.pytorch.org/whl/cpu
         pip install -r requirements/requirements_dev.txt
 
+    - name: Debug typing environment
+      run: |
+        python --version
+        mypy --version
+        pip freeze | grep -E '^(mypy|types-)'
+
     - name: Perform type checking
       run: |
-        mypy --install-types --non-interactive --follow-imports=skip \
+        mypy --install-types --non-interactive --follow-imports=silent \
           tiatoolbox/__init__.py \
           tiatoolbox/__main__.py \
           tiatoolbox/type_hints.py \

tests/test_utils.py

@@ -4,6 +4,7 @@
 
 import hashlib
 import json
+import re
 import shutil
 from pathlib import Path
 from typing import TYPE_CHECKING, NoReturn
@@ -42,6 +43,7 @@
     cast_to_min_dtype,
     create_smart_array,
     dict_to_store_patch_predictions,
+    imread,
 )
 from tiatoolbox.utils.transforms import locsize2bounds
 
@@ -1877,6 +1879,12 @@ def test_torch_compile_disable() -> None:
     assert model == compiled_model
 
 
+def test_torch_compile_none() -> None:
+    """Test torch_compile with a non-model input."""
+    with pytest.raises(ValueError, match=re.escape("`model` must not be None.")):
+        compile_model(model=None)
+
+
 def test_torch_compile_compatibility(caplog: pytest.LogCaptureFixture) -> None:
     """Test if torch-compile compatibility is checked correctly."""
     is_torch_compile_compatible()
@@ -2457,3 +2465,29 @@ def test_dict_to_store_patch_predictions_returns_qupath_json() -> None:
         assert feature["class_value"] in class_dict
         assert feature["properties"]["classification"]["name"] in class_dict.values()
         assert feature["properties"]["classification"]["color"] is not None
+
+
+def test_imread_invalid_path() -> None:
+    """Test imread with an invalid file path."""
+    invalid_path = "non_existent_image.jpg"
+    with pytest.raises(
+        FileNotFoundError, match=re.escape(f"Image path does not exist: {invalid_path}")
+    ):
+        imread(invalid_path)
+
+
+def test_imread_cv2_fails(track_tmp_path: Path) -> None:
+    """Test imread when cv2 fails to read an existing image file."""
+    # Create a temporary file that exists but contains invalid image data
+    tmp_image_path = track_tmp_path / "invalid_image.jpg"
+    with tmp_image_path.open("wb") as tmp:
+        tmp.write(b"invalid image data")
+
+    try:
+        with pytest.raises(
+            OSError, match=re.escape(f"Cannot read image: {tmp_image_path}")
+        ):
+            imread(tmp_image_path)
+    finally:
+        # Clean up the temporary file
+        tmp_image_path.unlink()

tiatoolbox/models/architecture/utils.py

@@ -78,6 +78,10 @@ def compile_model(
             Compiled model.
 
     """
+    if model is None:
+        msg = "`model` must not be None."
+        raise ValueError(msg)
+
     if mode == "disable":
         return model
 
@@ -321,6 +325,8 @@ def __init__(self, name: str | None, in_channels: int, reduction: int = 16) -> N
         """
         super().__init__()
 
+        self.attention: nn.Module
+
         if name is None:
             self.attention = nn.Identity()
         elif name == "scse":

tiatoolbox/tools/graph.py

@@ -384,11 +384,11 @@ def build(
             i_vs_all_similarities[neighbour_indexes_single_point] = (
                 neighbour_similarities
             )
-            i_vs_all_similarities = i_vs_all_similarities[i + 1 :]
-            condensed_distance_matrix[index : index + len(i_vs_all_similarities)] = (
-                i_vs_all_similarities
-            )
-            index = index + len(i_vs_all_similarities)
+            i_vs_all_similarities_tail = i_vs_all_similarities[i + 1 :]
+            condensed_distance_matrix[
+                index : index + len(i_vs_all_similarities_tail)
+            ] = i_vs_all_similarities_tail
+            index = index + len(i_vs_all_similarities_tail)
 
         # Perform hierarchical clustering (using similarity as distance)
         linkage_matrix = hierarchy.linkage(condensed_distance_matrix, method="average")

tiatoolbox/tools/patchextraction.py

@@ -3,7 +3,7 @@
 from __future__ import annotations
 
 from abc import ABC, abstractmethod
-from typing import TYPE_CHECKING, TypedDict, overload
+from typing import TYPE_CHECKING, TypedDict, cast, overload
 
 import numpy as np
 from typing_extensions import Unpack
@@ -432,7 +432,10 @@ def filter_coordinates(
             0,
             tissue_mask.shape[0],
         )
-        scaled_coords_list = list((scaled_coords).astype(np.int32))
+        scaled_coords_list = cast(
+            "list[list[int]]",
+            scaled_coords.astype(np.int32).tolist(),
+        )
 
         def default_sel_func(
             tissue_mask: np.ndarray,

tiatoolbox/utils/image.py

@@ -647,7 +647,7 @@ def sub_pixel_read(  # skipcq: PY-R1000  # noqa: C901, PLR0912, PLR0913, PLR0915
         ],
     )
     residuals = np.abs(int_read_bounds - read_bounds)
-    read_bounds = int_read_bounds
+    read_bounds = tuple(int_read_bounds)
     read_location, read_size = bounds2locsize(int_read_bounds)
     valid_int_bounds: np.ndarray = find_overlap(
         read_location=read_location,

tiatoolbox/utils/misc.py

@@ -197,10 +197,17 @@ def imread(image_path: PathLike, *, as_uint8: bool | None = None) -> np.ndarray:
     if isinstance(image_path, str):
         image_path = Path(image_path)
 
+    if not image_path.exists():
+        msg = f"Image path does not exist: {image_path}"
+        raise FileNotFoundError(msg)
+
     if image_path.suffix == ".npy":
         image = np.load(str(image_path))
     else:
         image = cv2.imread(str(image_path))
+        if image is None:
+            msg = f"Cannot read image: {image_path}"
+            raise OSError(msg)
         image = cv2.cvtColor(image, cv2.COLOR_BGR2RGB)
     if as_uint8:
         return image.astype(np.uint8)
@@ -1468,11 +1475,13 @@ def dict_to_store_semantic_segmentor(
 
     ignore_index = -1 if ignore_index is None else ignore_index
     # Get the number of unique predictions
-    layer_list = da.unique(preds).compute()
-    layer_list = np.delete(layer_list, np.where(layer_list == ignore_index))
+    layer_list_np = da.unique(preds).compute()
+    layer_list = (
+        np.delete(layer_list_np, np.where(layer_list_np == ignore_index))
+    ).tolist()
 
     if class_dict is None:
-        class_dict = {int(i): int(i) for i in layer_list.tolist()}
+        class_dict = {int(i): int(i) for i in layer_list}
 
     if output_type.lower() == "qupath":
         return _semantic_segmentations_as_qupath_json(
@@ -1939,9 +1948,14 @@ def cast_to_min_dtype(array: np.ndarray | da.Array) -> np.ndarray | da.Array:
     if max_value == 1:
         return array.astype(bool)
 
-    dtypes = [np.uint8, np.uint16, np.uint32, np.uint64]
-    for dtype in dtypes:
-        if max_value <= np.iinfo(dtype).max:
+    dtype_candidates = (
+        (np.iinfo(np.uint8).max, np.uint8),
+        (np.iinfo(np.uint16).max, np.uint16),
+        (np.iinfo(np.uint32).max, np.uint32),
+        (np.iinfo(np.uint64).max, np.uint64),
+    )
+    for max_allowed, dtype in dtype_candidates:
+        if max_value <= max_allowed:
             return array.astype(dtype)
 
     return array