diff --git a/.github/workflows/prepare_test_data.yaml b/.github/workflows/prepare_test_data.yaml index 14475c66..a72da9d6 100644 --- a/.github/workflows/prepare_test_data.yaml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 82bbd8e8..6521a5dc 100644 --- a/CHANGELOG.md +++ b/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 - `. - ### Removed - Support for Python 3.11. diff --git a/pyproject.toml b/pyproject.toml index 41f06caf..9c7135e1 100644 --- a/pyproject.toml +++ b/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]] diff --git a/src/spatialdata_io/readers/dbit.py b/src/spatialdata_io/readers/dbit.py index 51f96bd8..3bb744b7 100644 --- a/src/spatialdata_io/readers/dbit.py +++ b/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}") diff --git a/src/spatialdata_io/readers/macsima.py b/src/spatialdata_io/readers/macsima.py index cb7b3e75..20d9f859 100644 --- a/src/spatialdata_io/readers/macsima.py +++ b/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: diff --git a/src/spatialdata_io/readers/merscope.py b/src/spatialdata_io/readers/merscope.py index aa703c35..d454d741 100644 --- a/src/spatialdata_io/readers/merscope.py +++ b/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( diff --git a/tests/test_dbit.py b/tests/test_dbit.py new file mode 100644 index 00000000..6aa32280 --- /dev/null +++ b/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 diff --git a/tests/test_macsima.py b/tests/test_macsima.py index f557f8f4..e46613d2 100644 --- a/tests/test_macsima.py +++ b/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 diff --git a/tests/test_merscope.py b/tests/test_merscope.py new file mode 100644 index 00000000..715bdc4e --- /dev/null +++ b/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 diff --git a/tests/test_visium.py b/tests/test_visium.py new file mode 100644 index 00000000..3dad176f --- /dev/null +++ b/tests/test_visium.py @@ -0,0 +1,161 @@ +import json +import math +from pathlib import Path +from tempfile import TemporaryDirectory + +import numpy as np +import pandas as pd +import pytest +from click.testing import CliRunner +from shapely import Point +from spatialdata import SpatialData, get_extent, read_zarr +from spatialdata.models import ShapesModel, get_table_keys +from spatialdata.transformations import get_transformation + +from spatialdata_io.__main__ import visium_wrapper +from spatialdata_io._constants._constants import VisiumKeys +from spatialdata_io.readers.visium import visium + +# --- END-TO-END TESTS ON EXAMPLE DATA --- +# This dataset name is used to locate the test data in the './data/' directory. +# See https://github.com/scverse/spatialdata-io/blob/main/.github/workflows/prepare_test_data.yaml +# for instructions on how to download and place the data on disk. +DATASET_FOLDER = "CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer" +# the reader infers the same value from the name of the counts file +DATASET_ID = "CytAssist_FFPE_Protein_Expression_Human_Breast_Cancer" + +if not (Path("./data") / DATASET_FOLDER).is_dir(): + pytest.skip( + f"Requires the {DATASET_FOLDER} dataset (10x Genomics Space Ranger 2.1.0). The files and the " + "layout they are expected in are listed in .github/workflows/prepare_test_data.yaml.", + allow_module_level=True, + ) + + +@pytest.fixture(scope="module") +def dataset_path() -> Path: + return Path("./data") / DATASET_FOLDER + + +@pytest.fixture(scope="module") +def sdata(dataset_path: Path) -> SpatialData: + return visium(dataset_path, dataset_id=DATASET_ID) + + +def test_visium_elements(sdata: SpatialData) -> None: + """The reader builds the two downscaled images, the spots and the table.""" + assert list(sdata.images) == [f"{DATASET_ID}_hires_image", f"{DATASET_ID}_lowres_image"] + assert list(sdata.shapes) == [DATASET_ID] + assert list(sdata.tables) == ["table"] + # no full resolution image is passed, so `_full_image` is not created + assert f"{DATASET_ID}_full_image" not in sdata.images + assert sorted(sdata.coordinate_systems) == sorted( + [DATASET_ID, f"{DATASET_ID}_downscaled_hires", f"{DATASET_ID}_downscaled_lowres"] + ) + assert sdata.attrs["spatialdata_io_reader"] == "visium" + + +@pytest.mark.parametrize( + "coordinate_system,expected", + [ + (DATASET_ID, {"y": (0, 22630), "x": (-125, 23128)}), + (f"{DATASET_ID}_downscaled_hires", {"y": (0, 1957), "x": (-11, 2000)}), + (f"{DATASET_ID}_downscaled_lowres", {"y": (0, 587), "x": (-4, 600)}), + ], +) +def test_visium_data_extent(sdata: SpatialData, coordinate_system: str, expected: dict[str, tuple[int, int]]) -> None: + """Each coordinate system covers the image, plus the spots that fall outside of it.""" + extent = get_extent(sdata, exact=False, coordinate_system=coordinate_system) + extent = {ax: (math.floor(extent[ax][0]), math.ceil(extent[ax][1])) for ax in extent} + assert extent == expected + + +def test_visium_images(sdata: SpatialData) -> None: + """The two downscaled images are read as RGB, and each lives in its own coordinate system.""" + hires = sdata[f"{DATASET_ID}_hires_image"] + lowres = sdata[f"{DATASET_ID}_lowres_image"] + + for image in (hires, lowres): + assert image.dims == ("c", "y", "x") + assert image.dtype == np.uint8 + assert image.coords["c"].values.tolist() == ["r", "g", "b"] + + assert hires.shape == (3, 1957, 2000) + assert lowres.shape == (3, 587, 600) + assert np.array_equal(hires.data[:, 1000, 1000].compute(), [8, 5, 92]) + assert np.array_equal(lowres.data[:, 300, 300].compute(), [13, 8, 63]) + + # each image is unscaled in its own coordinate system, and scaled down in the one of the full resolution image + assert sorted(get_transformation(hires, get_all=True)) == sorted([DATASET_ID, f"{DATASET_ID}_downscaled_hires"]) + assert sorted(get_transformation(lowres, get_all=True)) == sorted([DATASET_ID, f"{DATASET_ID}_downscaled_lowres"]) + + +def test_visium_table(sdata: SpatialData) -> None: + """The table holds the filtered counts, annotated by the spots.""" + table = sdata["table"] + + assert table.shape == (4169, 18085) + assert table.obs_names[:3].tolist() == ["AACACTTGGCAAGGAA-1", "AACAGGATTCATAGTT-1", "AACAGGCCAACGATTA-1"] + assert table.var_names[:3].tolist() == ["SAMD11", "NOC2L", "KLHL17"] + assert table.obs_names.is_unique + assert table.var_names.is_unique + assert np.array_equal(table.X.indices[:3], [3, 6, 7]) + + # the spot coordinates are moved to `obsm`, the remaining columns of `tissue_positions` are kept + assert table.obs.columns.tolist() == ["in_tissue", "array_row", "array_col", "spot_id", "region"] + assert table.obs["spot_id"].tolist() == list(range(len(table))) + # the filtered matrix only contains the spots under the tissue + assert table.obs["in_tissue"].eq(1).all() + + assert get_table_keys(table) == (DATASET_ID, "region", "spot_id") + + +def test_visium_circles_are_the_spot_coordinates(sdata: SpatialData, dataset_path: Path) -> None: + """The circles are the spot coordinates of the table, in the order of the table.""" + circles = sdata[DATASET_ID] + table = sdata["table"] + + # ground truth, read independently of the reader and reordered to follow the table + positions = pd.read_csv(dataset_path / "spatial" / VisiumKeys.SPOTS_FILE_2, index_col=0) + expected = positions.loc[table.obs_names, [VisiumKeys.SPOTS_X, VisiumKeys.SPOTS_Y]].to_numpy() + + assert np.array_equal(circles.get_coordinates().to_numpy(), expected) + assert np.array_equal(table.obsm["spatial"], expected) + assert circles.index.tolist() == table.obs["spot_id"].tolist() + assert all(isinstance(geometry, Point) for geometry in circles.geometry) + + scalefactors = json.loads((dataset_path / "spatial" / VisiumKeys.SCALEFACTORS_FILE).read_bytes()) + expected_radius = scalefactors["spot_diameter_fullres"] / 2.0 + assert np.array_equal(circles[ShapesModel.RADIUS_KEY], np.full(len(table), expected_radius)) + + +def test_visium_dataset_id_is_inferred_from_the_counts_file(dataset_path: Path, sdata: SpatialData) -> None: + """Without `dataset_id` the elements are named after the prefix of the counts file.""" + inferred = visium(dataset_path) + + assert list(inferred.images) == list(sdata.images) + assert list(inferred.shapes) == list(sdata.shapes) + assert sorted(inferred.coordinate_systems) == sorted(sdata.coordinate_systems) + + +# --- CLI WRAPPER TEST --- + + +def test_cli_visium(runner: CliRunner, dataset_path: Path) -> None: + """The reader is reachable from the command line and the result can be written and read back.""" + with TemporaryDirectory() as tmpdir: + output_zarr = Path(tmpdir) / "data.zarr" + result = runner.invoke( + visium_wrapper, + [ + "--input", + str(dataset_path), + "--output", + str(output_zarr), + ], + ) + assert result.exit_code == 0, result.output + + sdata = read_zarr(output_zarr) + assert list(sdata.shapes) == [DATASET_ID] + assert sdata["table"].shape == (4169, 18085)