Fix four behaviour changes introduced by the mypy adoption in #411

.github/workflows/prepare_test_data.yaml

@@ -52,6 +52,19 @@ jobs:
           # 10x Genomics Xenium 4.0.0 (v1+Protein) Human kidney, multimodal cell segmentation
           curl -O https://cf.10xgenomics.com/samples/xenium/4.0.0/Xenium_V1_Protein_Human_Kidney_tiny/Xenium_V1_Protein_Human_Kidney_tiny_outs.zip
 
+          # -------
+          # the Visium dataset is licensed as CC BY 4.0, as shown here
+          # https://www.10xgenomics.com/datasets/gene-and-protein-expression-library-of-human-breast-cancer-cytassist-ffpe-2-standard
+
+          # 10x Genomics Visium CytAssist Gene and Protein Expression Library of Human Breast Cancer, IF, 6.5mm (FFPE)
+          mkdir -p CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer
+          cd CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer
+          # The full-resolution tissue image is deliberately not downloaded (~2 GB): `fullres_image_file`
+          # is optional and the reader path it exercises is shared with the other readers.
+          curl -O https://cf.10xgenomics.com/samples/spatial-exp/2.1.0/CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer/CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer_filtered_feature_bc_matrix.h5
+          curl -O https://cf.10xgenomics.com/samples/spatial-exp/2.1.0/CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer/CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer_spatial.tar.gz
+          cd ..
+
           # -------
           # the Visium HD dataset is licensed as CC BY 4.0, as shown here
           # https://www.10xgenomics.com/support/software/space-ranger/latest/resources/visium-hd-example-data
@@ -81,6 +94,11 @@ jobs:
               unzip "$file" -d "$dir"
               rm "$file"
           done
+          # the Visium archive contains a single `spatial/` directory, extracted next to the `.h5`
+          for file in */*.tar.gz; do
+              tar -xzf "$file" -C "$(dirname "$file")"
+              rm "$file"
+          done
 
       - name: Upload artifacts
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2

CHANGELOG.md

@@ -23,12 +23,6 @@ Release notes for `v0.7.1` and earlier are available on the [Releases][] page.
   documentation builds, `mypy` type checking of `src` and `tests`, `biome`/`pyproject-fmt`/`zizmor` pre-commit hooks,
   and Dependabot updates.
 
-### Fixed
-
-- `visium()`: the circles are built again from the spot coordinates instead of from the raw `tissue_positions` table,
-  which made the reader raise `TypeError: ShapesModel.parse() does not support the type
-  <class 'pandas.core.frame.DataFrame'>`.
-
 ### Removed
 
 - Support for Python 3.11.

pyproject.toml

@@ -146,6 +146,11 @@ lint.per-file-ignores."docs/*" = [ "I" ]
 lint.per-file-ignores."tests/*" = [ "D" ]
 lint.pydocstyle.convention = "numpy"
 
+[tool.mypy]
+# `/tests/data/` is one of the gitignored locations the test datasets are downloaded to; it is
+# not source of ours, and it can hold whatever scripts were used to prepare the data.
+exclude = "^tests/data/"
+
 # Dependencies that ship neither inline types nor stubs. Listed explicitly rather than
 # globally, so that a newly added untyped dependency is still reported.
 [[tool.mypy.overrides]]

src/spatialdata_io/readers/dbit.py

@@ -27,7 +27,7 @@
 
 
 def _check_path(
-    path: Path,
+    path: Path | None,
     pattern: Pattern[str],
     key: DbitKeys,
     path_specific: str | Path | None = None,
@@ -85,6 +85,8 @@ def _check_path(
                 raise FileNotFoundError(f"{path_specific} is not a valid path for a {key} file.")
 
     else:
+        if path is None:
+            raise ValueError(f"Either `path` or a specific path for the {key} file must be provided.")
         # search for the pattern matching file in path
         matches = [i for i in os.listdir(path) if pattern.match(i)]
         if len(matches) > 1:
@@ -266,12 +268,13 @@ def dbit(
     -------
     :class:`spatialdata.SpatialData`.
     """
-    path = Path() if path is None else Path(path)
-    # if path is invalid, raise error
-    if not os.path.isdir(path):
-        raise FileNotFoundError(
-            f"The path you have passed: {path} has not been found. A correct path to the data directory is needed."
-        )
+    if path is not None:
+        path = Path(path)
+        # if path is invalid, raise error
+        if not os.path.isdir(path):
+            raise FileNotFoundError(
+                f"The path you have passed: {path} has not been found. A correct path to the data directory is needed."
+            )
 
     # compile regex pattern to find file name in path, according to _constants.DbitKeys()
     patt_h5ad = re.compile(f".*{DbitKeys.COUNTS_FILE}")

src/spatialdata_io/readers/macsima.py

@@ -83,7 +83,10 @@ def from_paths(
         for p in path_files:
             try:
                 metadata = parse_metadata(p)
-            except ValueError as e:
+            # `path_files` may contain any file the user left in the folder, and the failure modes
+            # of `from_tiff()` are not enumerable (e.g. `struct.error` for a truncated header), so
+            # every file whose metadata cannot be parsed is skipped
+            except Exception as e:  # noqa: BLE001
                 warnings.warn(
                     f"Cannot parse OME metadata from {p}. Error: {e}. Skipping this file.",
                     UserWarning,
@@ -798,7 +801,10 @@ def create_sdata(
         for p in path_files:
             try:
                 pixels_to_microns = parse_physical_size(p)
-            except (OSError, ValueError, IndexError, NotImplementedError):
+            # `path_files` may contain anything, including truncated or non-OME files: the failure
+            # modes of `from_tiff()` are not enumerable (e.g. `struct.error` for a truncated
+            # header), so every file that cannot be parsed is skipped
+            except Exception:  # noqa: BLE001
                 logger.debug(f"Could not parse physical size from {p}. Trying next file.")
                 continue
         if pixels_to_microns is None:

src/spatialdata_io/readers/merscope.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-import importlib.util
+import importlib
 import re
 import warnings
 from pathlib import Path
@@ -236,9 +236,13 @@ def merscope(
 def _get_reader(backend: str | None) -> Callable[..., Image2DModel]:
     if backend is not None:
         return _rioxarray_load_merscope if backend == "rioxarray" else _dask_image_load_merscope
-    if importlib.util.find_spec("rioxarray") is not None:
-        return _rioxarray_load_merscope
-    return _dask_image_load_merscope
+    # `find_spec` only reports whether the module can be *found*: importing it can still fail,
+    # e.g. when `rasterio` is broken, and in that case we want the `dask_image` backend
+    try:
+        importlib.import_module("rioxarray")
+    except ImportError:
+        return _dask_image_load_merscope
+    return _rioxarray_load_merscope
 
 
 def _rioxarray_load_merscope(

tests/test_dbit.py

@@ -0,0 +1,33 @@
+import re
+from pathlib import Path
+
+import pytest
+
+from spatialdata_io._constants._constants import DbitKeys
+from spatialdata_io.readers.dbit import _check_path
+
+
+def test_check_path_without_a_directory_raises() -> None:
+    """Without a directory to search, `_check_path` should raise an exception."""
+    with pytest.raises(ValueError, match="Either `path` or a specific path"):
+        _check_path(
+            path=None,
+            pattern=re.compile(f".*{DbitKeys.COUNTS_FILE}"),
+            key=DbitKeys.COUNTS_FILE,
+        )
+
+
+def test_check_path_uses_the_specific_path_without_a_directory(tmp_path: Path) -> None:
+    """A file given explicitly is used even when no directory is given."""
+    counts_file = tmp_path / f"counts{DbitKeys.COUNTS_FILE}"
+    counts_file.touch()
+
+    file_path, flag = _check_path(
+        path=None,
+        pattern=re.compile(f".*{DbitKeys.COUNTS_FILE}"),
+        key=DbitKeys.COUNTS_FILE,
+        path_specific=counts_file,
+    )
+
+    assert file_path == counts_file
+    assert flag

tests/test_macsima.py

@@ -1,6 +1,7 @@
 import contextlib
 import math
 import os
+import re
 import shutil
 from copy import deepcopy
 from pathlib import Path
@@ -913,3 +914,22 @@ def test_parse_ome_metadata_unknown_major_raises() -> None:
 
     with pytest.raises(ValueError, match="Unknown software version"):
         _parse_ome_metadata(ome)
+
+
+def test_macsima_skips_files_whose_physical_size_cannot_be_parsed(tmp_path: Path) -> None:
+    """A single unreadable file in the folder must not abort the reader.
+
+    `path_files` can contain anything the user left in the folder, and the failure modes of
+    `ome_types.from_tiff()` are not enumerable: a truncated TIFF header raises `struct.error`.
+    """
+    dataset = tmp_path / "OMAP10_small"
+    shutil.copytree("./data/OMAP10_small", dataset)
+    reference = sorted(dataset.glob("*.tif"))[0]
+    # a TIFF with a valid header but truncated before the metadata
+    truncated = dataset / "C-099_S-000_S_APC_R-01_W-C-1_ROI-01_A-Junk_C-JUNK.tif"
+    truncated.write_bytes(reference.read_bytes()[:200])
+
+    with pytest.warns(UserWarning, match=re.escape(f"Cannot parse OME metadata from {truncated}")):
+        sdata = macsima(dataset, subset=32, c_subset=4, multiscale=False)
+
+    assert "OMAP10_small_image" in sdata.images

tests/test_merscope.py

@@ -0,0 +1,32 @@
+import sys
+from pathlib import Path
+
+import pytest
+
+from spatialdata_io.readers.merscope import (
+    _dask_image_load_merscope,
+    _get_reader,
+    _rioxarray_load_merscope,
+)
+
+
+@pytest.fixture
+def broken_rioxarray(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    """Shadow `rioxarray` with a module that can be found but not imported.
+
+    This is what a `rioxarray` installation with a broken `rasterio` looks like.
+    """
+    (tmp_path / "rioxarray.py").write_text("raise ModuleNotFoundError(\"No module named 'rasterio'\")\n")
+    monkeypatch.syspath_prepend(str(tmp_path))
+    monkeypatch.delitem(sys.modules, "rioxarray", raising=False)
+
+
+def test_get_reader_honours_an_explicit_backend() -> None:
+    assert _get_reader("rioxarray") is _rioxarray_load_merscope
+    assert _get_reader("dask_image") is _dask_image_load_merscope
+
+
+@pytest.mark.usefixtures("broken_rioxarray")
+def test_get_reader_falls_back_when_rioxarray_cannot_be_imported() -> None:
+    """A `rioxarray` that is installed but raises on import must not select the rioxarray backend."""
+    assert _get_reader(None) is _dask_image_load_merscope