diff --git a/.github/workflows/prepare_test_data.yaml b/.github/workflows/prepare_test_data.yaml index 34d15cca..92d20666 100644 --- a/.github/workflows/prepare_test_data.yaml +++ b/.github/workflows/prepare_test_data.yaml @@ -2,82 +2,67 @@ name: Prepare test data on: schedule: - - cron: "0 0 1 * *" # run once a month to prevent artifact expiration + - cron: "0 0 1 */2 *" # Run on the first day of every other month at midnight workflow_dispatch: - # Uncomment and adjust the branch name if you need to add new datasets to the artifact. - # It needs to be a branch in the spatialdata-io origin repository, not from a fork. -# push: -# branches: -# - main + inputs: + force_all: + description: "Download all registered datasets. Set to false to use dataset_keys." + required: true + type: boolean + default: true + dataset_keys: + description: "Dataset keys to download when force_all is false. Separate keys with spaces or commas." + required: false + type: string + default: "" + force_redownload: + description: "Redownload and replace existing selected datasets." + required: true + type: boolean + default: false + push: + branches: + - main + paths: + - ".github/workflows/prepare_test_data.yaml" + - "scripts/test_data_downloader/**" jobs: prepare-data: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - name: Checkout repository + uses: actions/checkout@v6 - - name: Download test datasets - run: | - mkdir -p ./data - cd ./data - - # ------- - # the Xenium datasets are licensed as CC BY 4.0, as shown here - # https://www.10xgenomics.com/support/software/xenium-onboard-analysis/latest/resources/xenium-example-data - - # 10x Genomics Xenium 2.0.0 - curl -O https://cf.10xgenomics.com/samples/xenium/2.0.0/Xenium_V1_human_Breast_2fov/Xenium_V1_human_Breast_2fov_outs.zip - curl -O https://cf.10xgenomics.com/samples/xenium/2.0.0/Xenium_V1_human_Lung_2fov/Xenium_V1_human_Lung_2fov_outs.zip - - # 10x Genomics Xenium 3.0.0 (5K) Mouse ileum, multimodal cell segmentation - # this file seems to be corrupted; skipping it for now - # curl -O https://cf.10xgenomics.com/samples/xenium/3.0.0/Xenium_Prime_MultiCellSeg_Mouse_Ileum_tiny/Xenium_Prime_MultiCellSeg_Mouse_Ileum_tiny.zip - - # 10x Genomics Xenium 3.0.0 (5K) Mouse ileum, nuclear expansion - curl -O https://cf.10xgenomics.com/samples/xenium/3.0.0/Xenium_Prime_Mouse_Ileum_tiny/Xenium_Prime_Mouse_Ileum_tiny_outs.zip - - # 10x Genomics Xenium 4.0.0 (v1) Human ovary, nuclear expansion - curl -O https://cf.10xgenomics.com/samples/xenium/4.0.0/Xenium_V1_Human_Ovary_tiny/Xenium_V1_Human_Ovary_tiny_outs.zip - - # 10x Genomics Xenium 4.0.0 (v1) Human ovary, multimodal cell segmentation - curl -O https://cf.10xgenomics.com/samples/xenium/4.0.0/Xenium_V1_MultiCellSeg_Human_Ovary_tiny/Xenium_V1_MultiCellSeg_Human_Ovary_tiny_outs.zip - - # 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 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 - - # 10x Genomics Visium HD 4.0.1 3' Mouse Brain Chunk - curl -O https://cf.10xgenomics.com/samples/spatial-exp/4.0.1/Visium_HD_Tiny_3prime_Dataset/Visium_HD_Tiny_3prime_Dataset_outs.zip - - # ------- - # we received written permission to make the following dataset public and integrate it in the CI system of spatialdata-io - # Spatial Genomics seqFISH v2 - curl -O https://s3.embl.de/spatialdata/raw_data/seqfish-2-test-dataset.zip - - # ------- - # MACSima OMAP datasets are licensed as CC BY 4.0 - # OMAP23 for format v1.x.x - curl -o OMAP23_small.zip "https://zenodo.org/api/records/18196452/files-archive" - - # OMAP10 for format v0.x.x - curl -o OMAP10_small.zip "https://zenodo.org/api/records/18196366/files-archive" + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.13" - - name: Unzip files + - name: Download test datasets run: | - cd ./data - for file in *.zip; do - dir="${file%.zip}" - mkdir -p "$dir" - unzip "$file" -d "$dir" - rm "$file" - done + args=(--output ./data) + if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.force_all }}" == "false" ]]; then + dataset_keys="${{ inputs.dataset_keys }}" + if [[ -z "${dataset_keys}" ]]; then + echo "::error::dataset_keys must be provided when force_all is false." + exit 1 + fi + dataset_keys="${dataset_keys//,/ }" + for dataset_key in ${dataset_keys}; do + args+=(--dataset "${dataset_key}") + done + fi + if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.force_redownload }}" == "true" ]]; then + args+=(--force) + fi + python scripts/test_data_downloader "${args[@]}" - name: Upload artifacts - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: data path: ./data + if-no-files-found: error + retention-days: 64 diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 17cea589..2c17c872 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -9,9 +9,9 @@ jobs: runs-on: ubuntu-latest if: startsWith(github.ref, 'refs/tags/v') steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Set up Python 3.12 - uses: actions/setup-python@v5 + uses: actions/setup-python@v6 with: python-version: "3.13" cache: pip diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 4ee1a57d..f128a314 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -26,9 +26,9 @@ jobs: PYTHON: ${{ matrix.python }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Set up Python ${{ matrix.python }} - uses: actions/setup-python@v5 + uses: actions/setup-python@v6 with: python-version: ${{ matrix.python }} @@ -37,7 +37,7 @@ jobs: run: | echo "::set-output name=dir::$(pip cache dir)" - name: Restore pip cache - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: ${{ steps.pip-cache-dir.outputs.dir }} key: pip-${{ runner.os }}-${{ env.pythonLocation }}-${{ hashFiles('**/pyproject.toml') }} diff --git a/docs/contributing.md b/docs/contributing.md index ebfda601..372f671c 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -123,6 +123,38 @@ If the `download.py` and `to_zarr.py` scripts require Python imports for package We encourage testing the reader function and any helper function. +Tests are split by scope: + +- Unit tests live in `tests/unit/` and should not require downloaded test data. +- Integration tests live in `tests/integration/` and cover reader workflows, CLI commands, file I/O, and zarr roundtrips. +- Integration tests that require external datasets use dataset keys from `scripts/test_data_downloader/datasets.toml`. + They resolve data under `SPATIALDATA_IO_TEST_DATA_DIR` when set, otherwise `data/` in the repository root. If the required dataset is unavailable, the test should skip with a clear message. +- Reader tests are marked by reader name. When modifying one reader, use `pytest -m ` to run the tests + specific to that reader, including shared parametrized checks for that reader. + +Useful local commands: + +```bash +pytest tests/unit +pytest tests/integration +pytest -m "integration and data" +pytest -m xenium +pytest -m "xenium and data" +pytest -m "xenium and not slow" +pytest -m "xenium and cli" +python scripts/test_data_downloader --group xenium +SPATIALDATA_IO_TEST_DATA_DIR=/path/to/data pytest -m data +``` + +To download the same optional datasets used by CI, run: + +```bash +python scripts/test_data_downloader +``` + +By default, the downloader skips datasets that already exist. Use `--force` to redownload selected datasets, `--dataset` for a single dataset key, and `--list` to show the available keys. +The dataset registry lives in `scripts/test_data_downloader/datasets.toml`; append new entries there when adding or updating test datasets. + ### Testing multiple versions When multiple versions of the raw data format are present, we encourage testing the reader on all of them to ensure backward compatibility. This task is greatly simplified if small test datasets are used for the CI tests. If this is not available, we suggest running the tests locally on multiple versions of the data before the PR is ready for review. diff --git a/pyproject.toml b/pyproject.toml index 05c5baa2..038b5b30 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,6 +82,29 @@ testpaths = ["tests"] xfail_strict = true addopts = [ "--import-mode=importlib", # allow using test files with same name + "--strict-markers", +] +markers = [ + "unit: fast isolated tests that do not require external datasets", + "integration: multi-component tests, file I/O tests, or reader workflow tests", + "data: tests that require optional downloaded test datasets", + "slow: tests with comparatively high runtime", + "cli: command-line interface tests", + "codex: tests for the codex reader", + "cosmx: tests for the cosmx reader", + "curio: tests for the curio reader", + "dbit: tests for the dbit reader", + "generic: tests for the generic reader module", + "iss: tests for the iss reader", + "macsima: tests for the macsima reader", + "mcmicro: tests for the mcmicro reader", + "merscope: tests for the merscope reader", + "seqfish: tests for the seqfish reader", + "steinbock: tests for the steinbock reader", + "stereoseq: tests for the stereoseq reader", + "visium: tests for the visium reader", + "visium_hd: tests for the visium_hd reader", + "xenium: tests for the xenium reader", ] [tool.ruff] diff --git a/scripts/test_data_downloader/__main__.py b/scripts/test_data_downloader/__main__.py new file mode 100644 index 00000000..b63fb357 --- /dev/null +++ b/scripts/test_data_downloader/__main__.py @@ -0,0 +1,8 @@ +"""Command-line entrypoint for the optional test data downloader.""" + +from __future__ import annotations + +from downloader import main + +if __name__ == "__main__": + main() diff --git a/scripts/test_data_downloader/datasets.toml b/scripts/test_data_downloader/datasets.toml new file mode 100644 index 00000000..b578489d --- /dev/null +++ b/scripts/test_data_downloader/datasets.toml @@ -0,0 +1,106 @@ +# ------- +# the Xenium datasets are licensed as CC BY 4.0, as shown here +# https://www.10xgenomics.com/support/software/xenium-onboard-analysis/latest/resources/xenium-example-data + +# 10x Genomics Xenium 2.0.0 +[[datasets]] +key = "xenium_breast" +group = "xenium" +url = "https://cf.10xgenomics.com/samples/xenium/2.0.0/Xenium_V1_human_Breast_2fov/Xenium_V1_human_Breast_2fov_outs.zip" +archive_name = "Xenium_V1_human_Breast_2fov_outs.zip" +extracted_dir = "Xenium_V1_human_Breast_2fov_outs" +source = "10x Genomics Xenium 2.0.0, CC BY 4.0" + +# 10x Genomics Xenium 2.0.0 +[[datasets]] +key = "xenium_lung" +group = "xenium" +url = "https://cf.10xgenomics.com/samples/xenium/2.0.0/Xenium_V1_human_Lung_2fov/Xenium_V1_human_Lung_2fov_outs.zip" +archive_name = "Xenium_V1_human_Lung_2fov_outs.zip" +extracted_dir = "Xenium_V1_human_Lung_2fov_outs" +source = "10x Genomics Xenium 2.0.0, CC BY 4.0" + +# 10x Genomics Xenium 3.0.0 (5K) Mouse ileum, multimodal cell segmentation +# this file seems to be corrupted; skipping it for now +# https://cf.10xgenomics.com/samples/xenium/3.0.0/Xenium_Prime_MultiCellSeg_Mouse_Ileum_tiny/Xenium_Prime_MultiCellSeg_Mouse_Ileum_tiny.zip + +# 10x Genomics Xenium 3.0.0 (5K) Mouse ileum, nuclear expansion +[[datasets]] +key = "xenium_prime_mouse_ileum" +group = "xenium" +url = "https://cf.10xgenomics.com/samples/xenium/3.0.0/Xenium_Prime_Mouse_Ileum_tiny/Xenium_Prime_Mouse_Ileum_tiny_outs.zip" +archive_name = "Xenium_Prime_Mouse_Ileum_tiny_outs.zip" +extracted_dir = "Xenium_Prime_Mouse_Ileum_tiny_outs" +source = "10x Genomics Xenium 3.0.0, CC BY 4.0" + +# 10x Genomics Xenium 4.0.0 (v1) Human ovary, nuclear expansion +[[datasets]] +key = "xenium_ovary" +group = "xenium" +url = "https://cf.10xgenomics.com/samples/xenium/4.0.0/Xenium_V1_Human_Ovary_tiny/Xenium_V1_Human_Ovary_tiny_outs.zip" +archive_name = "Xenium_V1_Human_Ovary_tiny_outs.zip" +extracted_dir = "Xenium_V1_Human_Ovary_tiny_outs" +source = "10x Genomics Xenium 4.0.0, CC BY 4.0" + +# 10x Genomics Xenium 4.0.0 (v1) Human ovary, multimodal cell segmentation +[[datasets]] +key = "xenium_multicell_ovary" +group = "xenium" +url = "https://cf.10xgenomics.com/samples/xenium/4.0.0/Xenium_V1_MultiCellSeg_Human_Ovary_tiny/Xenium_V1_MultiCellSeg_Human_Ovary_tiny_outs.zip" +archive_name = "Xenium_V1_MultiCellSeg_Human_Ovary_tiny_outs.zip" +extracted_dir = "Xenium_V1_MultiCellSeg_Human_Ovary_tiny_outs" +source = "10x Genomics Xenium 4.0.0, CC BY 4.0" + +# 10x Genomics Xenium 4.0.0 (v1+Protein) Human kidney, multimodal cell segmentation +[[datasets]] +key = "xenium_protein_kidney" +group = "xenium" +url = "https://cf.10xgenomics.com/samples/xenium/4.0.0/Xenium_V1_Protein_Human_Kidney_tiny/Xenium_V1_Protein_Human_Kidney_tiny_outs.zip" +archive_name = "Xenium_V1_Protein_Human_Kidney_tiny_outs.zip" +extracted_dir = "Xenium_V1_Protein_Human_Kidney_tiny_outs" +source = "10x Genomics Xenium 4.0.0, CC BY 4.0" + +# ------- +# 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 + +# 10x Genomics Visium HD 4.0.1 3' Mouse Brain Chunk +[[datasets]] +key = "visium_hd_tiny" +group = "visium_hd" +url = "https://cf.10xgenomics.com/samples/spatial-exp/4.0.1/Visium_HD_Tiny_3prime_Dataset/Visium_HD_Tiny_3prime_Dataset_outs.zip" +archive_name = "Visium_HD_Tiny_3prime_Dataset_outs.zip" +extracted_dir = "Visium_HD_Tiny_3prime_Dataset_outs" +source = "10x Genomics Visium HD 4.0.1, CC BY 4.0" + +# ------- +# we received written permission to make the following dataset public and integrate it in the CI system of spatialdata-io +# Spatial Genomics seqFISH v2 +[[datasets]] +key = "seqfish" +group = "seqfish" +url = "https://s3.embl.de/spatialdata/raw_data/seqfish-2-test-dataset.zip" +archive_name = "seqfish-2-test-dataset.zip" +extracted_dir = "seqfish-2-test-dataset" +source = "Spatial Genomics seqFISH v2, public test data" +test_path = "instrument 2 official" + +# ------- +# MACSima OMAP datasets are licensed as CC BY 4.0 +# OMAP23 for format v1.x.x +[[datasets]] +key = "macsima_omap23" +group = "macsima" +url = "https://zenodo.org/api/records/18196452/files-archive" +archive_name = "OMAP23_small.zip" +extracted_dir = "OMAP23_small" +source = "MACSima OMAP23, CC BY 4.0" + +# OMAP10 for format v0.x.x +[[datasets]] +key = "macsima_omap10" +group = "macsima" +url = "https://zenodo.org/api/records/18196366/files-archive" +archive_name = "OMAP10_small.zip" +extracted_dir = "OMAP10_small" +source = "MACSima OMAP10, CC BY 4.0" diff --git a/scripts/test_data_downloader/downloader.py b/scripts/test_data_downloader/downloader.py new file mode 100644 index 00000000..67fabd82 --- /dev/null +++ b/scripts/test_data_downloader/downloader.py @@ -0,0 +1,208 @@ +"""Download optional external datasets used by integration tests.""" + +from __future__ import annotations + +import argparse +import shutil +import tempfile +import time +import urllib.error +import urllib.request +import zipfile +from pathlib import Path +from typing import TYPE_CHECKING + +from manifest import DATASETS, TestDataset, validate_datasets + +if TYPE_CHECKING: + from collections.abc import Sequence + + +# Some providers reject Python's default user agent even for public files. +REQUEST_HEADERS = { + "User-Agent": "curl/8.0.0", + "Accept": "*/*", +} +DOWNLOAD_TIMEOUT_SEC = 60 +MAX_DOWNLOAD_ATTEMPTS = 3 +RETRY_DELAY_SEC = 2.0 +TRANSIENT_HTTP_STATUS_CODES = {408, 429} + + +class DatasetDownloadError(RuntimeError): + """Error raised when a test dataset cannot be downloaded or extracted.""" + + def __init__(self, dataset: TestDataset, action: str, reason: str, suggestion: str | None = None) -> None: + self.dataset = dataset + self.action = action + self.reason = reason + self.suggestion = suggestion + message = f"{dataset.key}: failed to {action} {dataset.url}: {reason}" + if suggestion is not None: + message = f"{message}. {suggestion}" + super().__init__(message) + + +def download_dataset(dataset: TestDataset, output: Path, force: bool) -> None: + """Download and extract a single dataset. + + Parameters + ---------- + dataset : TestDataset + Manifest entry describing the dataset archive and expected output + directory. + output : Path + Parent directory where the extracted dataset directory should live. + force : bool + If ``True``, replace an existing extracted dataset directory. + + Raises + ------ + DatasetDownloadError + If the archive cannot be downloaded, extracted, or validated. + """ + target = output / dataset.extracted_dir + if target.exists() and not force: + print(f"Skipping {dataset.key}: {target} already exists") + return + + output.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(prefix=f"{dataset.key}-", dir=output) as tmpdir: + tmp_path = Path(tmpdir) + archive_path = tmp_path / dataset.archive_name + extracted_path = tmp_path / dataset.extracted_dir + extracted_path.mkdir() + + # Stage work in a temporary directory so interrupted downloads do not leave partial datasets. + _download(dataset, archive_path) + _extract_zip(dataset, archive_path, extracted_path) + if not extracted_path.is_dir() or not any(extracted_path.iterdir()): + raise DatasetDownloadError( + dataset, + "extract", + f"archive did not produce expected directory {dataset.extracted_dir!r}", + ) + + if target.exists(): + shutil.rmtree(target) + # Move the fully validated extraction into place only after all checks pass. + shutil.move(str(extracted_path), target) + print(f"Downloaded {dataset.key} to {target}") + + +def main(argv: Sequence[str] | None = None) -> None: + """Run the dataset downloader command-line interface.""" + validate_datasets(DATASETS) + args = _parse_args(argv) + if args.list: + for dataset in DATASETS: + print(f"{dataset.key}\t{dataset.group}\t{dataset.extracted_dir}\t{dataset.source}") + return + + failures: list[DatasetDownloadError] = [] + for dataset in _selected_datasets(args.dataset, args.group): + try: + download_dataset(dataset, args.output, args.force) + except DatasetDownloadError as exc: + failures.append(exc) + print(f"ERROR: {exc}") + + if failures: + print(f"Failed to download {len(failures)} dataset(s):") + for failure in failures: + print(f"- {failure}") + raise SystemExit(1) + + +def _parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: + """Parse command-line arguments for selecting and downloading datasets.""" + dataset_keys = sorted(dataset.key for dataset in DATASETS) + groups = sorted({dataset.group for dataset in DATASETS}) + parser = argparse.ArgumentParser(description="Download optional test datasets used by spatialdata-io CI.") + parser.add_argument("--output", type=Path, default=Path("data"), help="Directory where datasets are extracted.") + parser.add_argument( + "--dataset", + action="append", + choices=dataset_keys, + help="Dataset key to download. May be passed multiple times. Defaults to all datasets.", + ) + parser.add_argument( + "--group", + action="append", + choices=groups, + help="Dataset group to download. May be passed multiple times.", + ) + parser.add_argument("--force", action="store_true", help="Redownload and replace existing selected datasets.") + parser.add_argument("--list", action="store_true", help="List available datasets and exit.") + return parser.parse_args(argv) + + +def _selected_datasets(dataset_keys: list[str] | None, groups: list[str] | None) -> list[TestDataset]: + """Return manifest entries selected by explicit keys, groups, or all datasets.""" + selected_keys = set(dataset_keys or ()) + selected_groups = set(groups or ()) + if not selected_keys and not selected_groups: + return list(DATASETS) + return [dataset for dataset in DATASETS if dataset.key in selected_keys or dataset.group in selected_groups] + + +def _is_transient_error(error: BaseException) -> bool: + """Return whether a download error is worth retrying.""" + if isinstance(error, urllib.error.HTTPError): + return error.code in TRANSIENT_HTTP_STATUS_CODES or 500 <= error.code < 600 + return isinstance(error, urllib.error.URLError | TimeoutError) + + +def _download_error(dataset: TestDataset, error: BaseException) -> DatasetDownloadError: + """Translate urllib and timeout failures into dataset-aware errors.""" + if isinstance(error, urllib.error.HTTPError): + reason = f"HTTP {error.code} {error.reason}" + suggestion = None + if error.code == 403: + suggestion = ( + "The server rejected the request. The downloader sends a curl-like User-Agent; " + "if this persists, verify the URL in a browser or with curl and check whether the provider changed access rules" + ) + return DatasetDownloadError(dataset, "download", reason, suggestion) + if isinstance(error, urllib.error.URLError): + return DatasetDownloadError(dataset, "download", f"URL error: {error.reason}") + if isinstance(error, TimeoutError): + return DatasetDownloadError(dataset, "download", f"timed out after {DOWNLOAD_TIMEOUT_SEC} seconds") + return DatasetDownloadError(dataset, "download", str(error)) + + +def _download(dataset: TestDataset, archive_path: Path) -> None: + """Download ``dataset`` to ``archive_path`` with bounded retries.""" + print(f"Downloading {dataset.key} from {dataset.url}") + for attempt in range(1, MAX_DOWNLOAD_ATTEMPTS + 1): + request = urllib.request.Request(dataset.url, headers=REQUEST_HEADERS) + try: + with urllib.request.urlopen(request, timeout=DOWNLOAD_TIMEOUT_SEC) as response: + with archive_path.open("wb") as handle: + shutil.copyfileobj(response, handle) + return + except (urllib.error.HTTPError, urllib.error.URLError, TimeoutError) as exc: + if attempt < MAX_DOWNLOAD_ATTEMPTS and _is_transient_error(exc): + print(f"Retrying {dataset.key} after download failure ({attempt}/{MAX_DOWNLOAD_ATTEMPTS}): {exc}") + time.sleep(RETRY_DELAY_SEC) + continue + raise _download_error(dataset, exc) from exc + + +def _extract_zip(dataset: TestDataset, archive_path: Path, destination: Path) -> None: + """Extract ``archive_path`` into ``destination`` after validating member paths.""" + try: + with zipfile.ZipFile(archive_path) as archive: + destination_root = destination.resolve() + for member in archive.infolist(): + # Guard against zip-slip archives that contain absolute paths or ``..`` components. + member_path = (destination / member.filename).resolve() + if not member_path.is_relative_to(destination_root): + raise DatasetDownloadError( + dataset, + "extract", + f"archive member {member.filename!r} would be extracted outside {destination}", + ) + archive.extractall(destination) + except zipfile.BadZipFile as exc: + raise DatasetDownloadError(dataset, "extract", "downloaded file is not a valid zip archive") from exc diff --git a/scripts/test_data_downloader/manifest.py b/scripts/test_data_downloader/manifest.py new file mode 100644 index 00000000..843564f5 --- /dev/null +++ b/scripts/test_data_downloader/manifest.py @@ -0,0 +1,164 @@ +"""Dataset manifest loading and validation for optional test data downloads.""" + +from __future__ import annotations + +import tomllib +from dataclasses import dataclass +from functools import cache +from pathlib import Path, PurePath +from typing import Any + +DATASETS_TOML = Path(__file__).with_name("datasets.toml") +REQUIRED_FIELDS = frozenset({"key", "group", "url", "archive_name", "extracted_dir", "source"}) +OPTIONAL_FIELDS = frozenset({"test_path"}) +ALLOWED_FIELDS = REQUIRED_FIELDS | OPTIONAL_FIELDS + + +@dataclass(frozen=True) +class TestDataset: + """Test dataset to be downloaded. + + Parameters + ---------- + key : str + Unique identifier for the dataset, used for referencing in tests and + the CLI. + group : str + Logical grouping of the dataset, for example ``"xenium"``, + ``"visium_hd"``, ``"seqfish"``, or ``"macsima"``. + url : str + Direct URL to the dataset archive, for example a ZIP file. + archive_name : str + Expected filename of the downloaded archive. + extracted_dir : str + Expected name of the directory created when the archive is extracted. + source : str + Human-readable description of the dataset source and license. + test_path : str + Optional path inside the extracted directory that should be passed to + integration tests. + """ + + key: str + group: str + url: str + archive_name: str + extracted_dir: str + source: str + test_path: str = "" + + +def load_datasets(path: str | Path = DATASETS_TOML) -> tuple[TestDataset, ...]: + """Load dataset entries from a TOML manifest.""" + manifest_path = Path(path) + try: + raw_manifest = tomllib.loads(manifest_path.read_text(encoding="utf-8")) + except tomllib.TOMLDecodeError as exc: + raise ValueError(f"Invalid dataset manifest TOML in {manifest_path}: {exc}") from exc + + unknown_root_fields = set(raw_manifest) - {"datasets"} + if unknown_root_fields: + unknown = ", ".join(sorted(unknown_root_fields)) + raise ValueError(f"Dataset manifest has unknown root field(s): {unknown}.") + + raw_datasets = raw_manifest.get("datasets") + if not isinstance(raw_datasets, list): + raise ValueError("Dataset manifest must define a [[datasets]] array.") + + datasets = tuple(_parse_dataset(raw_dataset, index) for index, raw_dataset in enumerate(raw_datasets)) + validate_datasets(datasets) + return datasets + + +def validate_datasets(datasets: tuple[TestDataset, ...] | None = None) -> None: + """Validate that dataset entries are internally consistent. + + Parameters + ---------- + datasets : tuple[TestDataset, ...] + Dataset entries to validate. + + Raises + ------ + ValueError + If a required field is empty, a key or extracted directory is + duplicated, or ``test_path`` points outside the extracted directory. + """ + if datasets is None: + datasets = DATASETS + + seen_keys: set[str] = set() + seen_extracted_dirs: set[str] = set() + + for dataset in datasets: + for field_name in REQUIRED_FIELDS: + value = getattr(dataset, field_name) + if not isinstance(value, str): + raise ValueError(f"Dataset {dataset.key!r} field {field_name!r} must be a string.") + if not value.strip(): + raise ValueError(f"Dataset {dataset.key!r} has empty {field_name}.") + if not isinstance(dataset.test_path, str): + raise ValueError(f"Dataset {dataset.key!r} field 'test_path' must be a string.") + + test_path = PurePath(dataset.test_path) + # Test paths are appended to extracted_dir by tests, so they must stay inside that directory. + if dataset.test_path and (test_path.is_absolute() or ".." in test_path.parts): + raise ValueError(f"Dataset {dataset.key!r} test_path must be a relative path inside extracted_dir.") + if dataset.key in seen_keys: + raise ValueError(f"Duplicate test dataset key: {dataset.key!r}.") + if dataset.extracted_dir in seen_extracted_dirs: + raise ValueError(f"Duplicate test dataset extracted_dir: {dataset.extracted_dir!r}.") + seen_keys.add(dataset.key) + seen_extracted_dirs.add(dataset.extracted_dir) + + +def get_dataset(key: str) -> TestDataset: + """Return the dataset registered for ``key``. + + Raises + ------ + KeyError + If ``key`` is not a registered dataset key. + """ + try: + return _datasets_by_key()[key] + except KeyError as exc: + available = ", ".join(sorted(_datasets_by_key())) + raise KeyError(f"Unknown test dataset key {key!r}. Available keys: {available}") from exc + + +def datasets_by_group(group: str) -> tuple[TestDataset, ...]: + """Return datasets registered for ``group`` in manifest order.""" + return tuple(dataset for dataset in DATASETS if dataset.group == group) + + +def _parse_dataset(raw_dataset: object, index: int) -> TestDataset: + if not isinstance(raw_dataset, dict): + raise ValueError(f"Dataset manifest entry at index {index} must be a table.") + + dataset_fields = set(raw_dataset) + unknown_fields = dataset_fields - ALLOWED_FIELDS + if unknown_fields: + unknown = ", ".join(sorted(unknown_fields)) + raise ValueError(f"Dataset manifest entry at index {index} has unknown field(s): {unknown}.") + + missing_fields = REQUIRED_FIELDS - dataset_fields + if missing_fields: + missing = ", ".join(sorted(missing_fields)) + raise ValueError(f"Dataset manifest entry at index {index} is missing required field(s): {missing}.") + + values: dict[str, Any] = dict(raw_dataset) + values.setdefault("test_path", "") + dataset = TestDataset(**values) + validate_datasets((dataset,)) + return dataset + + +# Keep keys and groups stable because CI workflows and pytest markers may refer to them. +DATASETS = load_datasets() + + +@cache +def _datasets_by_key() -> dict[str, TestDataset]: + """Return cached lookup table for dataset keys.""" + return {dataset.key: dataset for dataset in DATASETS} diff --git a/tests/_utils.py b/tests/_utils.py deleted file mode 100644 index 5087b900..00000000 --- a/tests/_utils.py +++ /dev/null @@ -1,30 +0,0 @@ -import sys - -import pytest - - -def skip_if_below_python_version() -> pytest.mark.skipif: - """Decorator to skip tests if the Python version is below a specified version. - - This decorator prevents running tests on unsupported Python versions. Update the `MIN_VERSION` - constant to change the minimum Python version required for the tests. - - Returns - ------- - pytest.mark.skipif - A pytest marker that skips the test if the current Python version is below the specified `MIN_VERSION`. - - Notes - ----- - The current minimum version is set to Python 3.10. Adjust the `MIN_VERSION` constant as needed - to accommodate newer Python versions. - - Examples - -------- - >>> @skip_if_below_python_version() - >>> def test_some_feature(): - >>> assert True - """ - MIN_VERSION = (3, 13) - reason = f"Test requires Python {'.'.join(map(str, MIN_VERSION))} or higher" - return pytest.mark.skipif(sys.version_info < MIN_VERSION, reason=reason) diff --git a/tests/conftest.py b/tests/conftest.py index 5e4c26ad..278a208a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,97 @@ +from __future__ import annotations + +import importlib.util +import os +import sys +from pathlib import Path +from typing import TYPE_CHECKING, cast + import pytest from click.testing import CliRunner +if TYPE_CHECKING: + from collections.abc import Callable + from types import ModuleType + + from manifest import TestDataset as TestDatasetType + + +def _load_dataset_manifest() -> ModuleType: + manifest_path = Path(__file__).parents[1] / "scripts" / "test_data_downloader" / "manifest.py" + spec = importlib.util.spec_from_file_location("manifest", manifest_path) + if spec is None or spec.loader is None: + raise RuntimeError(f"Could not import {manifest_path}") + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +_DATASET_MANIFEST = _load_dataset_manifest() + +_READER_MARKS = { + "codex", + "cosmx", + "curio", + "dbit", + "generic", + "iss", + "macsima", + "mcmicro", + "merscope", + "seqfish", + "steinbock", + "stereoseq", + "visium", + "visium_hd", + "xenium", +} + @pytest.fixture def runner() -> CliRunner: return CliRunner() + + +@pytest.fixture(scope="session") +def test_data_dir() -> Path: + """Return the directory containing optional test datasets.""" + return Path(os.environ.get("SPATIALDATA_IO_TEST_DATA_DIR", "data")) + + +@pytest.fixture +def require_test_dataset(test_data_dir: Path) -> Callable[[str], Path]: + """Return a dataset path or skip the test if the dataset is unavailable.""" + + def _require_test_dataset(dataset_key: str) -> Path: + dataset = cast("TestDatasetType", _DATASET_MANIFEST.get_dataset(dataset_key)) + path: Path = test_data_dir / dataset.extracted_dir + if dataset.test_path: + path = path / dataset.test_path + if not path.is_dir(): + pytest.skip( + f"Test data for {dataset_key!r} not found at {path!s}. " + f"Download it with `uv run python scripts/test_data_downloader --dataset {dataset_key}` or set " + "SPATIALDATA_IO_TEST_DATA_DIR." + ) + return path + + return _require_test_dataset + + +def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: + """Apply scope and reader markers from the test path.""" + for item in items: + path = Path(str(item.fspath)) + parts = path.parts + if "unit" in parts: + item.add_marker(pytest.mark.unit) + if "integration" in parts: + item.add_marker(pytest.mark.integration) + if "cli" in parts or "cli" in item.name: + item.add_marker(pytest.mark.cli) + if "require_test_dataset" in getattr(item, "fixturenames", ()): + item.add_marker(pytest.mark.data) + for part in parts: + if part in _READER_MARKS: + item.add_marker(getattr(pytest.mark, part)) diff --git a/tests/test_generic.py b/tests/integration/readers/generic/test_generic.py similarity index 100% rename from tests/test_generic.py rename to tests/integration/readers/generic/test_generic.py diff --git a/tests/integration/readers/macsima/test_macsima.py b/tests/integration/readers/macsima/test_macsima.py new file mode 100644 index 00000000..9c1c598c --- /dev/null +++ b/tests/integration/readers/macsima/test_macsima.py @@ -0,0 +1,295 @@ +import math +import os +import shutil +from collections.abc import Callable +from pathlib import Path +from tempfile import TemporaryDirectory +from typing import Any + +import numpy as np +import pandas as pd +import pytest +from click.testing import CliRunner +from spatialdata import read_zarr +from spatialdata.models import get_channel_names +from tifffile import imwrite + +from spatialdata_io.__main__ import macsima_wrapper +from spatialdata_io.readers.macsima import macsima + + +def test_images_with_invalid_ome_metadata_are_excluded( + tmp_path: Path, require_test_dataset: Callable[[str], Path] +) -> None: + # Write a tiff file without metadata + # Use same dimensions as OMAP10_small, which we will use as a positive example + height = 77 + width = 94 + arr = np.zeros((height, width, 1), dtype=np.uint16) + path_no_metadata = Path(tmp_path) / "tiff_no_metadata.tiff" + imwrite(path_no_metadata, arr, metadata=None, description=None, software=None, datetime=None) + + # Copy 1 image from OMAP10 small + omap_10_image_path = ( + require_test_dataset("macsima_omap10") / "C-001_S-000_S_APC_R-01_W-C-1_ROI-01_A-CD15_C-VIMC6.tif" + ) + shutil.copy(omap_10_image_path, Path(tmp_path)) + + sdata = macsima(tmp_path) + el = sdata[list(sdata.images.keys())[0]] + channels = get_channel_names(el) + assert channels == ["CD15"] + + +def test_multiple_subfolder_parsing_skips_emtpy_folders( + tmp_path: Path, require_test_dataset: Callable[[str], Path] +) -> None: + parent_folder = tmp_path / "test_folder" + shutil.copytree(require_test_dataset("macsima_omap23"), parent_folder / "OMAP23_small") + os.makedirs(parent_folder / "empty_folder") + + with pytest.warns(UserWarning, match="No tif files found in .* skipping it"): + sdata = macsima(parent_folder, parsing_style="processed_multiple_folders") + assert len(sdata.images.keys()) == 1 + + +@pytest.mark.parametrize( + "dataset,expected", + [ + pytest.param("macsima_omap10", {"y": (0, 77), "x": (0, 94)}, id="macsima_omap10"), + pytest.param("macsima_omap23", {"y": (0, 77), "x": (0, 93)}, id="macsima_omap23"), + ], +) +def test_image_size(dataset: str, expected: dict[str, Any], require_test_dataset: Callable[[str], Path]) -> None: + from spatialdata import get_extent + + f = require_test_dataset(dataset) + sdata = macsima(f, transformations=False) # Do not transform to make it easier to compare against pixel dimensions + el = sdata[list(sdata.images.keys())[0]] + cs = sdata.coordinate_systems[0] + + extent: dict[str, tuple[float, float]] = get_extent(el, coordinate_system=cs) + extent = {ax: (math.floor(extent[ax][0]), math.ceil(extent[ax][1])) for ax in extent} + assert extent == expected + + +@pytest.mark.parametrize( + "dataset,expected", + [ + pytest.param("macsima_omap10", 4, id="macsima_omap10"), + pytest.param("macsima_omap23", 5, id="macsima_omap23"), + ], +) +def test_total_channels(dataset: str, expected: int, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) + sdata = macsima(f) + el = sdata[list(sdata.images.keys())[0]] + + # get the number of channels + channels: int = len(get_channel_names(el)) + assert channels == expected + + +@pytest.mark.parametrize( + "dataset,expected", + [ + pytest.param("macsima_omap10", ["R1 CD15", "R1 DAPI", "R2 Bcl 2", "R2 CD1c"], id="macsima_omap10"), + pytest.param( + "macsima_omap23", + ["R1 CD3", "R1 DAPI", "R2 CD279", "R4 CD66b", "R15 DAPI_background"], + id="macsima_omap23", + ), + ], +) +def test_channel_names_with_cycle_in_name( + dataset: str, expected: list[str], require_test_dataset: Callable[[str], Path] +) -> None: + f = require_test_dataset(dataset) + sdata = macsima(f, include_cycle_in_channel_name=True) + el = sdata[list(sdata.images.keys())[0]] + + # get the channel names + channels = get_channel_names(el) + assert list(channels) == expected + + +@pytest.mark.parametrize( + "dataset,expected", + [ + pytest.param("macsima_omap10", 2, id="macsima_omap10"), + pytest.param("macsima_omap23", 15, id="macsima_omap23"), + ], +) +def test_total_rounds(dataset: str, expected: list[int], require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) + sdata = macsima(f) + table = sdata[list(sdata.tables)[0]] + max_cycle = table.var["cycle"].max() + assert max_cycle == expected + + +@pytest.mark.parametrize( + "dataset,skip_rounds,expected", + [ + pytest.param("macsima_omap10", list(range(2, 4)), ["CD15", "DAPI"], id="macsima_omap10"), + pytest.param( + "macsima_omap23", + list(range(2, 16)), + ["CD3", "DAPI"], + id="macsima_omap23", + ), + ], +) +def test_skip_rounds( + dataset: str, skip_rounds: list[int], expected: list[str], require_test_dataset: Callable[[str], Path] +) -> None: + f = require_test_dataset(dataset) + sdata = macsima(f, skip_rounds=skip_rounds) + el = sdata[list(sdata.images.keys())[0]] + + # get the channel names + channels = get_channel_names(el) + assert list(channels) == expected, f"Expected {expected}, got {list(channels)}" + + +def test_processed_single_folder_parsing_returns_a_single_image_stack( + tmp_path: Path, require_test_dataset: Callable[[str], Path] +) -> None: + omap10_path = require_test_dataset("macsima_omap10") + shutil.copytree(omap10_path, tmp_path / "OMAP10_small_1") + shutil.copytree(omap10_path, tmp_path / "OMAP10_small_2") + + sdata = macsima(tmp_path, parsing_style="processed_single_folder") + + assert len(sdata.images) == 1 + # omap10_small has 4 channels, so we expect 8 here + el = sdata[list(sdata.images.keys())[0]] + assert len(get_channel_names(el)) == 8 + assert len(sdata.tables) == 1 + + +def test_processed_single_folder_parsing_warns_when_specifying_filtered_folders( + tmp_path: Path, require_test_dataset: Callable[[str], Path] +) -> None: + omap10_path = require_test_dataset("macsima_omap10") + shutil.copytree(omap10_path, tmp_path / "OMAP10_small_1") + shutil.copytree(omap10_path, tmp_path / "OMAP10_small_2") + with pytest.warns(UserWarning, match="filtering only happens for processed_multi_folders"): + macsima(tmp_path, parsing_style="processed_single_folder", filter_folder_names=["OMAP10_small_2"]) + + +def test_processed_multiple_folders_returns_an_image_stack_per_subfolder( + tmp_path: Path, require_test_dataset: Callable[[str], Path] +) -> None: + omap10_path = require_test_dataset("macsima_omap10") + shutil.copytree(omap10_path, tmp_path / "OMAP10_small_1") + shutil.copytree(omap10_path, tmp_path / "OMAP10_small_2") + + sdata = macsima(tmp_path, parsing_style="processed_multiple_folders") + + assert len(sdata.images) == 2 + for el in sdata.images.keys(): + assert len(get_channel_names(sdata[el])) == 4 + assert len(sdata.tables) == 2 + + +def test_processed_multiple_folders_skips_filtered_folder_names( + tmp_path: Path, require_test_dataset: Callable[[str], Path] +) -> None: + shutil.copytree(require_test_dataset("macsima_omap10"), tmp_path / "OMAP10_small") + shutil.copytree(require_test_dataset("macsima_omap23"), tmp_path / "OMAP23_small") + + sdata = macsima(tmp_path, parsing_style="processed_multiple_folders", filter_folder_names=["OMAP10_small"]) + assert len(sdata.images) == 1 + assert list(sdata.images.keys()) == ["OMAP23_small_image"] + assert len(sdata.tables) == 1 + assert list(sdata.tables.keys()) == ["OMAP23_small_table"] + + +METADATA_COLUMN_ORDER = [ + "cycle", + "imagetype", + "well", + "ROI", + "fluorophore", + "clone", + "exposure", +] + +EXPECTED_METADATA_OMAP10 = pd.DataFrame( + { + "name": ["CD15", "DAPI", "Bcl 2", "CD1c"], + "cycle": [1, 1, 2, 2], + "imagetype": ["stain", "stain", "stain", "stain"], + "well": ["C-1", "C-1", "C-1", "C-1"], + "ROI": [1, 1, 1, 1], + "fluorophore": ["APC", "DAPI", "FITC", "PE"], + "clone": ["VIMC6", pd.NA, "REA872", "REA694"], + "exposure": [2304.0, 40.0, 96.0, 144.0], + }, + index=["CD15", "DAPI", "Bcl 2", "CD1c"], + columns=METADATA_COLUMN_ORDER, +) + +EXPECTED_METADATA_OMAP23 = pd.DataFrame( + { + "name": ["CD3", "DAPI", "CD279", "CD66b", "DAPI_background"], + "cycle": [1, 1, 2, 4, 15], + "imagetype": ["stain", "stain", "stain", "stain", "bleach"], + "well": ["D01", "D01", "D01", "D01", "D01"], + "ROI": [1, 1, 1, 1, 1], + "fluorophore": ["APC", "DAPI", "PE", "FITC", "DAPI"], + "clone": ["REA1151", pd.NA, "REA1165", "REA306", pd.NA], + "exposure": [1212.52, 51.0, 322.12, 856.68, 51.0], + }, + index=["CD3", "DAPI", "CD279", "CD66b", "DAPI_background"], + columns=METADATA_COLUMN_ORDER, +) + + +@pytest.mark.parametrize( + "dataset,expected_df", + [ + pytest.param("macsima_omap10", EXPECTED_METADATA_OMAP10, id="macsima_omap10"), + pytest.param("macsima_omap23", EXPECTED_METADATA_OMAP23, id="macsima_omap23"), + ], +) +def test_metadata_table(dataset: str, expected_df: pd.DataFrame, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) + sdata = macsima(f) + table = sdata[list(sdata.tables.keys())[0]] + + # Convert table.var to a DataFrame and align to expected columns + actual = table.var[METADATA_COLUMN_ORDER] + + pd.testing.assert_frame_equal(actual, expected_df) + + +@pytest.mark.parametrize( + "dataset", + [ + pytest.param("macsima_omap10", id="macsima_omap10"), + pytest.param("macsima_omap23", id="macsima_omap23"), + ], +) +def test_cli_macsima(runner: CliRunner, dataset: str, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) + with TemporaryDirectory() as tmpdir: + output_zarr = Path(tmpdir) / "data.zarr" + result = runner.invoke( + macsima_wrapper, + [ + "--input", + f, + "--output", + output_zarr, + "--subset", + 500, + "--c-subset", + 1, + "--multiscale", + False, + ], + ) + assert result.exit_code == 0, result.output + _ = read_zarr(output_zarr) diff --git a/tests/test_seqfish.py b/tests/integration/readers/seqfish/test_seqfish.py similarity index 70% rename from tests/test_seqfish.py rename to tests/integration/readers/seqfish/test_seqfish.py index b71bfa6a..7b51c6d8 100644 --- a/tests/test_seqfish.py +++ b/tests/integration/readers/seqfish/test_seqfish.py @@ -1,4 +1,5 @@ import math +from collections.abc import Callable from pathlib import Path from tempfile import TemporaryDirectory @@ -8,20 +9,24 @@ from spatialdata_io.__main__ import seqfish_wrapper from spatialdata_io.readers.seqfish import seqfish -from tests._utils import skip_if_below_python_version # 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 -@skip_if_below_python_version() @pytest.mark.parametrize( - "dataset,expected", [("seqfish-2-test-dataset/instrument 2 official", "{'y': (0, 108), 'x': (0, 108)}")] + "dataset,expected", + [pytest.param("seqfish", "{'y': (0, 108), 'x': (0, 108)}", id="seqfish")], ) @pytest.mark.parametrize("rois", [["Roi1"], None]) @pytest.mark.parametrize("cells_as_circles", [False, True]) -def test_example_data(dataset: str, expected: str, rois: list[int] | None, cells_as_circles: bool) -> None: - f = Path("./data") / dataset - assert f.is_dir() +def test_example_data( + dataset: str, + expected: str, + rois: list[int] | None, + cells_as_circles: bool, + require_test_dataset: Callable[[str], Path], +) -> None: + f = require_test_dataset(dataset) sdata = seqfish(f, cells_as_circles=cells_as_circles, rois=rois) from spatialdata import get_extent @@ -34,11 +39,9 @@ def test_example_data(dataset: str, expected: str, rois: list[int] | None, cells assert str(extent) == expected -@skip_if_below_python_version() -@pytest.mark.parametrize("dataset", ["seqfish-2-test-dataset/instrument 2 official"]) -def test_cli_seqfish(runner: CliRunner, dataset: str) -> None: - f = Path("./data") / dataset - assert f.is_dir() +@pytest.mark.parametrize("dataset", [pytest.param("seqfish", id="seqfish")]) +def test_cli_seqfish(runner: CliRunner, dataset: str, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) with TemporaryDirectory() as tmpdir: output_zarr = Path(tmpdir) / "data.zarr" result = runner.invoke( diff --git a/tests/test_visium_hd.py b/tests/integration/readers/visium_hd/test_visium_hd.py similarity index 61% rename from tests/test_visium_hd.py rename to tests/integration/readers/visium_hd/test_visium_hd.py index 41dfd890..a1b6357b 100644 --- a/tests/test_visium_hd.py +++ b/tests/integration/readers/visium_hd/test_visium_hd.py @@ -1,8 +1,8 @@ import math +from collections.abc import Callable from pathlib import Path from tempfile import TemporaryDirectory -import numpy as np import pytest from click.testing import CliRunner from spatialdata import get_extent, read_zarr @@ -10,54 +10,20 @@ from spatialdata_io.__main__ import visium_hd_wrapper from spatialdata_io._constants._constants import VisiumHDKeys -from spatialdata_io.readers.visium_hd import ( - _decompose_projective_matrix, - _projective_matrix_is_affine, - visium_hd, -) -from tests._utils import skip_if_below_python_version - -# --- UNIT TESTS FOR HELPER FUNCTIONS --- - - -def test_projective_matrix_is_affine() -> None: - """Test the affine matrix check function.""" - # An affine matrix should have [0, 0, 1] as its last row - affine_matrix = np.array([[2, 0.5, 10], [0.5, 2, 20], [0, 0, 1]]) - assert _projective_matrix_is_affine(affine_matrix) - - # A projective matrix is not affine if the last row is different - projective_matrix = np.array([[2, 0.5, 10], [0.5, 2, 20], [0.01, 0.02, 1]]) - assert not _projective_matrix_is_affine(projective_matrix) - - -def test_decompose_projective_matrix() -> None: - """Test the decomposition of a projective matrix into affine and shift components.""" - projective_matrix = np.array([[1, 2, 3], [4, 5, 6], [0.1, 0.2, 1]]) - affine, shift = _decompose_projective_matrix(projective_matrix) - - expected_affine = np.array([[1, 2, 3], [4, 5, 6], [0, 0, 1]]) - - # The affine component should be correctly extracted - assert np.allclose(affine, expected_affine) - # Recomposing the affine and shift matrices should yield the original projective matrix - assert np.allclose(affine @ shift, projective_matrix) - +from spatialdata_io.readers.visium_hd import visium_hd # --- END-TO-END TESTS ON EXAMPLE DATA --- -# This dataset name is used to locate the test data in the './data/' directory. +# This dataset key is used to locate the test data in the dataset manifest. # 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 = "Visium_HD_Tiny_3prime_Dataset_outs" +DATASET_KEY = "visium_hd_tiny" DATASET_ID = "visium_hd_tiny" -@skip_if_below_python_version() -def test_visium_hd_data_extent() -> None: +@pytest.mark.slow +def test_visium_hd_data_extent(require_test_dataset: Callable[[str], Path]) -> None: """Check the spatial extent of the loaded Visium HD data.""" - f = Path("./data") / DATASET_FOLDER - if not f.is_dir(): - pytest.skip(f"Test data not found at '{f}'. Skipping extent test.") + f = require_test_dataset(DATASET_KEY) sdata = visium_hd(f, dataset_id=DATASET_ID) extent = get_extent(sdata, exact=False, coordinate_system="visium_hd_tiny_downscaled_lowres") @@ -68,42 +34,53 @@ def test_visium_hd_data_extent() -> None: assert str(extent) == expected_extent -@skip_if_below_python_version() @pytest.mark.parametrize( "params", [ # Test case 1: Default binned data loading (squares) - { - "load_segmentations_only": False, - "load_nucleus_segmentations": False, - "bins_as_squares": True, - "annotate_table_by_labels": False, - "load_all_images": False, - }, + pytest.param( + { + "load_segmentations_only": False, + "load_nucleus_segmentations": False, + "bins_as_squares": True, + "annotate_table_by_labels": False, + "load_all_images": False, + }, + marks=pytest.mark.slow, + ), # Test case 2: Binned data as circles - { - "load_segmentations_only": False, - "load_nucleus_segmentations": False, - "bins_as_squares": False, - "annotate_table_by_labels": False, - "load_all_images": False, - }, + pytest.param( + { + "load_segmentations_only": False, + "load_nucleus_segmentations": False, + "bins_as_squares": False, + "annotate_table_by_labels": False, + "load_all_images": False, + }, + marks=pytest.mark.slow, + ), # Test case 3: Binned data with tables annotating labels instead of shapes - { - "load_segmentations_only": False, - "load_nucleus_segmentations": False, - "bins_as_squares": True, - "annotate_table_by_labels": True, - "load_all_images": False, - }, + pytest.param( + { + "load_segmentations_only": False, + "load_nucleus_segmentations": False, + "bins_as_squares": True, + "annotate_table_by_labels": True, + "load_all_images": False, + }, + marks=pytest.mark.slow, + ), # Test case 4: Load binned data AND all segmentations (cell + nucleus) - { - "load_segmentations_only": False, - "load_nucleus_segmentations": True, - "bins_as_squares": True, - "annotate_table_by_labels": False, - "load_all_images": False, - }, + pytest.param( + { + "load_segmentations_only": False, + "load_nucleus_segmentations": True, + "bins_as_squares": True, + "annotate_table_by_labels": False, + "load_all_images": False, + }, + marks=pytest.mark.slow, + ), # Test case 5: Load cell segmentations only { "load_segmentations_only": True, @@ -122,11 +99,9 @@ def test_visium_hd_data_extent() -> None: }, ], ) -def test_visium_hd_data_integrity(params: dict[str, bool]) -> None: +def test_visium_hd_data_integrity(params: dict[str, bool], require_test_dataset: Callable[[str], Path]) -> None: """Check the integrity of various components of the loaded SpatialData object.""" - f = Path("./data") / DATASET_FOLDER - if not f.is_dir(): - pytest.skip(f"Test data not found at '{f}'. Skipping integrity test.") + f = require_test_dataset(DATASET_KEY) sdata = visium_hd(f, dataset_id=DATASET_ID, **params) @@ -182,17 +157,14 @@ def test_visium_hd_data_integrity(params: dict[str, bool]) -> None: # --- CLI WRAPPER TEST --- -@skip_if_below_python_version() @pytest.mark.parametrize( "dataset", - ["Visium_HD_Tiny_3prime_Dataset_outs"], + [pytest.param("visium_hd_tiny", id="visium_hd_tiny")], ) -def test_cli_visium_hd(runner: CliRunner, dataset: str) -> None: +@pytest.mark.slow +def test_cli_visium_hd(runner: CliRunner, dataset: str, require_test_dataset: Callable[[str], Path]) -> None: """Test the command-line interface for the Visium HD reader.""" - f = Path("./data") / dataset[0] - - if not f.is_dir(): - pytest.skip(f"Test data not found at '{f}'. Skipping CLI test.") + f = require_test_dataset(dataset) with TemporaryDirectory() as tmpdir: output_zarr = Path(tmpdir) / "data.zarr" diff --git a/tests/test_xenium.py b/tests/integration/readers/xenium/test_xenium.py similarity index 58% rename from tests/test_xenium.py rename to tests/integration/readers/xenium/test_xenium.py index 17903888..70b07e4c 100644 --- a/tests/test_xenium.py +++ b/tests/integration/readers/xenium/test_xenium.py @@ -1,89 +1,41 @@ import math +from collections.abc import Callable from pathlib import Path from tempfile import TemporaryDirectory import numpy as np import pytest from click.testing import CliRunner -from pytest_mock import MockerFixture from spatialdata import match_table_to_element, read_zarr -from spatialdata.models import TableModel, get_table_keys +from spatialdata.models import get_table_keys from spatialdata_io.__main__ import xenium_wrapper -from spatialdata_io.readers.xenium import ( - _cell_id_str_from_prefix_suffix_uint32_reference, - cell_id_str_from_prefix_suffix_uint32, - prefix_suffix_uint32_from_cell_id_str, - xenium, -) -from tests._utils import skip_if_below_python_version - - -def test_cell_id_str_from_prefix_suffix_uint32() -> None: - cell_id_prefix = np.array([1, 1437536272, 1437536273], dtype=np.uint32) - dataset_suffix = np.array([1, 1, 2]) - expected = np.array(["aaaaaaab-1", "ffkpbaba-1", "ffkpbabb-2"]) - - result = cell_id_str_from_prefix_suffix_uint32(cell_id_prefix, dataset_suffix) - reference = _cell_id_str_from_prefix_suffix_uint32_reference(cell_id_prefix, dataset_suffix) - assert np.array_equal(result, expected) - assert np.array_equal(reference, expected) - - -def test_cell_id_str_optimized_matches_reference() -> None: - rng = np.random.default_rng(42) - cell_id_prefix = rng.integers(0, 2**32, size=10_000, dtype=np.uint32) - dataset_suffix = rng.integers(0, 10, size=10_000) - - result = cell_id_str_from_prefix_suffix_uint32(cell_id_prefix, dataset_suffix) - reference = _cell_id_str_from_prefix_suffix_uint32_reference(cell_id_prefix, dataset_suffix) - assert np.array_equal(result, reference) - - -def test_prefix_suffix_uint32_from_cell_id_str() -> None: - cell_id_str = np.array(["aaaaaaab-1", "ffkpbaba-1", "ffkpbabb-2"]) - - cell_id_prefix, dataset_suffix = prefix_suffix_uint32_from_cell_id_str(cell_id_str) - assert np.array_equal(cell_id_prefix, np.array([1, 1437536272, 1437536273], dtype=np.uint32)) - assert np.array_equal(dataset_suffix, np.array([1, 1, 2])) - - -def test_roundtrip_with_data_limits() -> None: - # min and max values for uint32 - cell_id_prefix = np.array([0, 4294967295], dtype=np.uint32) - dataset_suffix = np.array([1, 1]) - cell_id_str = np.array(["aaaaaaaa-1", "pppppppp-1"]) - f0 = cell_id_str_from_prefix_suffix_uint32 - f1 = prefix_suffix_uint32_from_cell_id_str - assert np.array_equal(cell_id_prefix, f1(f0(cell_id_prefix, dataset_suffix))[0]) - assert np.array_equal(dataset_suffix, f1(f0(cell_id_prefix, dataset_suffix))[1]) - assert np.array_equal(cell_id_str, f0(*f1(cell_id_str))) +from spatialdata_io.readers.xenium import xenium # 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 # TODO: add tests for Xenium 3.0.0 -@skip_if_below_python_version() @pytest.mark.parametrize( "dataset,expected", [ ( - "Xenium_V1_human_Breast_2fov_outs", + "xenium_breast", "{'y': (0, 3529), 'x': (0, 5792), 'z': (10, 25)}", ), ( - "Xenium_V1_human_Lung_2fov_outs", + "xenium_lung", "{'y': (0, 3553), 'x': (0, 5793), 'z': (7, 32)}", ), ( - "Xenium_V1_Protein_Human_Kidney_tiny_outs", + "xenium_protein_kidney", "{'y': (0, 6915), 'x': (0, 2963), 'z': (6, 22)}", ), ], + ids=["xenium_breast", "xenium_lung", "xenium_protein_kidney"], ) -def test_example_data_data_extent(dataset: str, expected: str) -> None: - f = Path("./data") / dataset - assert f.is_dir() +def test_example_data_data_extent(dataset: str, expected: str, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) sdata = xenium(f, cells_as_circles=False) from spatialdata import get_extent @@ -93,17 +45,19 @@ def test_example_data_data_extent(dataset: str, expected: str) -> None: # TODO: add tests for Xenium 3.0.0 -@skip_if_below_python_version() @pytest.mark.parametrize( "dataset", - ["Xenium_V1_human_Breast_2fov_outs", "Xenium_V1_human_Lung_2fov_outs", "Xenium_V1_Protein_Human_Kidney_tiny_outs"], + [ + pytest.param("xenium_breast", id="xenium_breast"), + pytest.param("xenium_lung", id="xenium_lung"), + pytest.param("xenium_protein_kidney", id="xenium_protein_kidney"), + ], ) -def test_example_data_index_integrity(dataset: str) -> None: - f = Path("./data") / dataset - assert f.is_dir() +def test_example_data_index_integrity(dataset: str, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) sdata = xenium(f, cells_as_circles=False) - if dataset == "Xenium_V1_human_Breast_2fov_outs": + if dataset == "xenium_breast": # fmt: off # test elements assert sdata["morphology_focus"]["scale0"]["image"].sel(c="DAPI", y=20.5, x=20.5).data.compute() == 94 @@ -130,7 +84,7 @@ def test_example_data_index_integrity(dataset: str) -> None: "aaaljapa-1", "aabhbgmg-1", ] - elif dataset == "Xenium_V1_human_Lung_2fov_outs": + elif dataset == "xenium_lung": # fmt: off # test elements assert sdata["morphology_focus"]["scale0"]["image"].sel(c="DAPI", y=0.5, x=2215.5).data.compute() == 1 @@ -158,7 +112,7 @@ def test_example_data_index_integrity(dataset: str) -> None: "aabdiein-1", ] else: - assert dataset == "Xenium_V1_Protein_Human_Kidney_tiny_outs" + assert dataset == "xenium_protein_kidney" # fmt: off # test elements assert sdata["morphology_focus"]["scale0"]["image"].sel(c="VISTA", y=2876.5, x=32.5).data.compute() == 99 @@ -188,14 +142,16 @@ def test_example_data_index_integrity(dataset: str) -> None: # TODO: add tests for Xenium 3.0.0 -@skip_if_below_python_version() @pytest.mark.parametrize( "dataset", - ["Xenium_V1_human_Breast_2fov_outs", "Xenium_V1_human_Lung_2fov_outs", "Xenium_V1_Protein_Human_Kidney_tiny_outs"], + [ + pytest.param("xenium_breast", marks=pytest.mark.slow, id="xenium_breast"), + pytest.param("xenium_lung", marks=pytest.mark.slow, id="xenium_lung"), + pytest.param("xenium_protein_kidney", marks=pytest.mark.slow, id="xenium_protein_kidney"), + ], ) -def test_cli_xenium(runner: CliRunner, dataset: str) -> None: - f = Path("./data") / dataset - assert f.is_dir() +def test_cli_xenium(runner: CliRunner, dataset: str, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) with TemporaryDirectory() as tmpdir: output_zarr = Path(tmpdir) / "data.zarr" result = runner.invoke( @@ -211,30 +167,28 @@ def test_cli_xenium(runner: CliRunner, dataset: str) -> None: _ = read_zarr(output_zarr) -@skip_if_below_python_version() @pytest.mark.parametrize( ( "dataset", "gex_only", ), [ - ("Xenium_V1_human_Lung_2fov_outs", False), - ("Xenium_V1_human_Lung_2fov_outs", True), - ("Xenium_V1_Human_Ovary_tiny_outs", False), - ("Xenium_V1_Human_Ovary_tiny_outs", True), - ("Xenium_V1_MultiCellSeg_Human_Ovary_tiny_outs", False), - ("Xenium_V1_MultiCellSeg_Human_Ovary_tiny_outs", True), - ("Xenium_V1_Protein_Human_Kidney_tiny_outs", False), - ("Xenium_V1_Protein_Human_Kidney_tiny_outs", True), + pytest.param("xenium_lung", False, id="xenium_lung_all_features"), + pytest.param("xenium_lung", True, id="xenium_lung_gex_only"), + pytest.param("xenium_ovary", False, id="xenium_ovary_all_features"), + pytest.param("xenium_ovary", True, id="xenium_ovary_gex_only"), + pytest.param("xenium_multicell_ovary", False, id="xenium_multicell_ovary_all_features"), + pytest.param("xenium_multicell_ovary", True, id="xenium_multicell_ovary_gex_only"), + pytest.param("xenium_protein_kidney", False, id="xenium_protein_kidney_all_features"), + pytest.param("xenium_protein_kidney", True, id="xenium_protein_kidney_gex_only"), ], ) -def test_xenium_other_feature_types(dataset: str, gex_only: bool) -> None: - f = Path("./data") / dataset - assert f.is_dir() +def test_xenium_other_feature_types(dataset: str, gex_only: bool, require_test_dataset: Callable[[str], Path]) -> None: + f = require_test_dataset(dataset) sdata = xenium(f, cells_as_circles=False, gex_only=gex_only) if gex_only: assert set(sdata["table"].var["feature_types"]) == {"Gene Expression"} - elif dataset == "Xenium_V1_human_Lung_2fov_outs": + elif dataset == "xenium_lung": assert set(sdata["table"].var["feature_types"]) == { "Deprecated Codeword", "Gene Expression", @@ -242,7 +196,7 @@ def test_xenium_other_feature_types(dataset: str, gex_only: bool) -> None: "Negative Control Probe", "Unassigned Codeword", } - elif dataset in {"Xenium_V1_Human_Ovary_tiny_outs", "Xenium_V1_MultiCellSeg_Human_Ovary_tiny_outs"}: + elif dataset in {"xenium_ovary", "xenium_multicell_ovary"}: assert set(sdata["table"].var["feature_types"]) == { "Gene Expression", "Genomic Control", @@ -250,7 +204,7 @@ def test_xenium_other_feature_types(dataset: str, gex_only: bool) -> None: "Negative Control Probe", "Unassigned Codeword", } - elif dataset == "Xenium_V1_Protein_Human_Kidney_tiny_outs": + elif dataset == "xenium_protein_kidney": assert set(sdata["table"].var["feature_types"]) == { "Gene Expression", "Genomic Control", @@ -270,57 +224,3 @@ def test_xenium_other_feature_types(dataset: str, gex_only: bool) -> None: else: assert ValueError(f"Unexpected dataset {dataset}") - - -# ── CLI JSON kwargs tests (no real data needed) ─────────────────────────────── - - -@pytest.mark.parametrize( - "kwarg_name", - ["--imread-kwargs", "--image-models-kwargs", "--labels-models-kwargs"], -) -def test_cli_xenium_invalid_json_rejected(runner: CliRunner, tmp_path: Path, kwarg_name: str) -> None: - """Invalid JSON for any kwargs option must produce a non-zero exit and a clear error.""" - result = runner.invoke( - xenium_wrapper, - [ - "--input", - str(tmp_path), - "--output", - str(tmp_path / "out.zarr"), - kwarg_name, - "not-valid-json{", - ], - ) - assert result.exit_code != 0 - assert "Invalid JSON" in result.output - - -@pytest.mark.parametrize( - ("kwarg_name", "kwarg_param"), - [ - ("--imread-kwargs", "imread_kwargs"), - ("--image-models-kwargs", "image_models_kwargs"), - ("--labels-models-kwargs", "labels_models_kwargs"), - ], -) -def test_cli_xenium_valid_json_forwarded( - runner: CliRunner, tmp_path: Path, mocker: MockerFixture, kwarg_name: str, kwarg_param: str -) -> None: - """Valid JSON kwargs must be parsed and forwarded to the xenium reader as a dict.""" - mock_xenium = mocker.patch("spatialdata_io.readers.xenium.xenium") - mock_xenium.return_value = mocker.MagicMock() - result = runner.invoke( - xenium_wrapper, - [ - "--input", - str(tmp_path), - "--output", - str(tmp_path / "out.zarr"), - kwarg_name, - '{"chunks": 512}', - ], - ) - assert result.exit_code == 0, result.output - call_kwargs = mock_xenium.call_args.kwargs - assert call_kwargs[kwarg_param] == {"chunks": 512} diff --git a/tests/test_cli_alignment.py b/tests/unit/cli/test_cli_alignment.py similarity index 77% rename from tests/test_cli_alignment.py rename to tests/unit/cli/test_cli_alignment.py index cab2c96a..9ea1345a 100644 --- a/tests/test_cli_alignment.py +++ b/tests/unit/cli/test_cli_alignment.py @@ -34,22 +34,27 @@ import pytest + +def _reader_case(module_path: str, reader_name: str, wrapper_name: str) -> Any: + return pytest.param(module_path, reader_name, wrapper_name, marks=getattr(pytest.mark, reader_name), id=reader_name) + + # (reader_module_path, reader_func_name, cli_wrapper_func_name) _READERS = [ - ("spatialdata_io.readers.codex", "codex", "codex_wrapper"), - ("spatialdata_io.readers.cosmx", "cosmx", "cosmx_wrapper"), - ("spatialdata_io.readers.curio", "curio", "curio_wrapper"), - ("spatialdata_io.readers.dbit", "dbit", "dbit_wrapper"), - ("spatialdata_io.readers.iss", "iss", "iss_wrapper"), - ("spatialdata_io.readers.macsima", "macsima", "macsima_wrapper"), - ("spatialdata_io.readers.mcmicro", "mcmicro", "mcmicro_wrapper"), - ("spatialdata_io.readers.merscope", "merscope", "merscope_wrapper"), - ("spatialdata_io.readers.seqfish", "seqfish", "seqfish_wrapper"), - ("spatialdata_io.readers.steinbock", "steinbock", "steinbock_wrapper"), - ("spatialdata_io.readers.stereoseq", "stereoseq", "stereoseq_wrapper"), - ("spatialdata_io.readers.visium", "visium", "visium_wrapper"), - ("spatialdata_io.readers.visium_hd", "visium_hd", "visium_hd_wrapper"), - ("spatialdata_io.readers.xenium", "xenium", "xenium_wrapper"), + _reader_case("spatialdata_io.readers.codex", "codex", "codex_wrapper"), + _reader_case("spatialdata_io.readers.cosmx", "cosmx", "cosmx_wrapper"), + _reader_case("spatialdata_io.readers.curio", "curio", "curio_wrapper"), + _reader_case("spatialdata_io.readers.dbit", "dbit", "dbit_wrapper"), + _reader_case("spatialdata_io.readers.iss", "iss", "iss_wrapper"), + _reader_case("spatialdata_io.readers.macsima", "macsima", "macsima_wrapper"), + _reader_case("spatialdata_io.readers.mcmicro", "mcmicro", "mcmicro_wrapper"), + _reader_case("spatialdata_io.readers.merscope", "merscope", "merscope_wrapper"), + _reader_case("spatialdata_io.readers.seqfish", "seqfish", "seqfish_wrapper"), + _reader_case("spatialdata_io.readers.steinbock", "steinbock", "steinbock_wrapper"), + _reader_case("spatialdata_io.readers.stereoseq", "stereoseq", "stereoseq_wrapper"), + _reader_case("spatialdata_io.readers.visium", "visium", "visium_wrapper"), + _reader_case("spatialdata_io.readers.visium_hd", "visium_hd", "visium_hd_wrapper"), + _reader_case("spatialdata_io.readers.xenium", "xenium", "xenium_wrapper"), ] # Parameters to skip in the reader (first positional path arg, and **kwargs catch-alls) diff --git a/tests/converters/__init__.py b/tests/unit/converters/__init__.py similarity index 100% rename from tests/converters/__init__.py rename to tests/unit/converters/__init__.py diff --git a/tests/converters/test_legacy_anndata.py b/tests/unit/converters/test_legacy_anndata.py similarity index 100% rename from tests/converters/test_legacy_anndata.py rename to tests/unit/converters/test_legacy_anndata.py diff --git a/tests/test_macsima.py b/tests/unit/readers/macsima/test_macsima.py similarity index 66% rename from tests/test_macsima.py rename to tests/unit/readers/macsima/test_macsima.py index 9be7a3c6..7f0634db 100644 --- a/tests/test_macsima.py +++ b/tests/unit/readers/macsima/test_macsima.py @@ -1,17 +1,11 @@ import contextlib -import math -import os -import shutil from copy import deepcopy from pathlib import Path -from tempfile import TemporaryDirectory from typing import Any import dask.array as da import numpy as np -import pandas as pd import pytest -from click.testing import CliRunner from ome_types import OME from ome_types.model import ( Image, @@ -26,11 +20,8 @@ StructuredAnnotations, Well, ) -from spatialdata import read_zarr -from spatialdata.models import get_channel_names from tifffile import imwrite -from spatialdata_io.__main__ import macsima_wrapper from spatialdata_io.readers.macsima import ( ChannelMetadata, MultiChannelImage, @@ -46,14 +37,6 @@ RNG = da.random.default_rng(seed=0) -if not (Path("./data/OMAP10_small").exists() or Path("./data/OMAP23_small").exists()): - pytest.skip( - "Requires the OMAP10 or OMAP23 datasets. " - "The small OMAP10 dataset can be downloaded from https://zenodo.org/api/records/18196366/files-archive, for the full data see https://zenodo.org/records/7875938" - "The small OMAP23 dataset can be downloaded from https://zenodo.org/api/records/18196452/files-archive, for the full data set see https://zenodo.org/records/14008816", - allow_module_level=True, - ) - # Helper to create ChannelMetadata with some defaults def make_ChannelMetadata( @@ -81,25 +64,6 @@ def make_ChannelMetadata( ) -def test_images_with_invalid_ome_metadata_are_excluded(tmp_path: Path) -> None: - # Write a tiff file without metadata - # Use same dimensions as OMAP10_small, which we will use as a positive example - height = 77 - width = 94 - arr = np.zeros((height, width, 1), dtype=np.uint16) - path_no_metadata = Path(tmp_path) / "tiff_no_metadata.tiff" - imwrite(path_no_metadata, arr, metadata=None, description=None, software=None, datetime=None) - - # Copy 1 image from OMAP10 small - omap_10_image_path = Path("./data") / "OMAP10_small" / "C-001_S-000_S_APC_R-01_W-C-1_ROI-01_A-CD15_C-VIMC6.tif" - shutil.copy(omap_10_image_path, Path(tmp_path)) - - sdata = macsima(tmp_path) - el = sdata[list(sdata.images.keys())[0]] - channels = get_channel_names(el) - assert channels == ["CD15"] - - def test_exception_on_no_valid_files(tmp_path: Path) -> None: # Write a tiff file without metadata height = 10 @@ -112,16 +76,6 @@ def test_exception_on_no_valid_files(tmp_path: Path) -> None: macsima(tmp_path) -def test_multiple_subfolder_parsing_skips_emtpy_folders(tmp_path: Path) -> None: - parent_folder = tmp_path / "test_folder" - shutil.copytree("./data/OMAP23_small", parent_folder / "OMAP23_small") - os.makedirs(parent_folder / "empty_folder") - - with pytest.warns(UserWarning, match="No tif files found in .* skipping it"): - sdata = macsima(parent_folder, parsing_style="processed_multiple_folders") - assert len(sdata.images.keys()) == 1 - - @pytest.mark.parametrize( "dimensions,expected", [ @@ -173,9 +127,7 @@ def test_padding_on_differing_dimensions() -> None: for height, width in zip(heights, widths, strict=True): arr = da.from_array(np.ones((1, height, width), dtype=np.uint16)) imgs.append(arr) - channel_metadata = channel_metadata = [ - make_ChannelMetadata(name="test", cycle=1, translation_x=100, translation_y=100) - ] * 4 + channel_metadata = [make_ChannelMetadata(name="test", cycle=1, translation_x=100, translation_y=100)] * 4 with pytest.warns(UserWarning, match="Padding images with 0s to same size of \\(20, 20\\)"): imgs_padded = MultiChannelImage._pad_images(imgs, channel_metadata) for img in imgs_padded: @@ -190,7 +142,7 @@ def test_padding_on_differing_dimensions() -> None: for height, width in zip(heights, widths, strict=True): arr = da.from_array(np.ones((1, height, width), dtype=np.uint16)) imgs.append(arr) - channel_metadata = channel_metadata = [ + channel_metadata = [ make_ChannelMetadata(name="test", cycle=1, translation_x=2, translation_y=3), make_ChannelMetadata(name="test", cycle=1, translation_x=0, translation_y=0), ] @@ -210,7 +162,7 @@ def test_padding_on_differing_dimensions() -> None: for height, width in zip(heights, widths, strict=True): arr = da.from_array(np.ones((1, height, width), dtype=np.uint16)) imgs.append(arr) - channel_metadata = channel_metadata = [ + channel_metadata = [ make_ChannelMetadata(name="test", cycle=1, translation_x=2, translation_y=3), make_ChannelMetadata(name="test", cycle=1, translation_x=5, translation_y=5), ] @@ -220,212 +172,11 @@ def test_padding_on_differing_dimensions() -> None: assert img.shape == (1, 17, 18) -@pytest.mark.parametrize( - "dataset,expected", - [ - ("OMAP10_small", {"y": (0, 77), "x": (0, 94)}), - ("OMAP23_small", {"y": (0, 77), "x": (0, 93)}), - ], -) -def test_image_size(dataset: str, expected: dict[str, Any]) -> None: - from spatialdata import get_extent - - f = Path("./data") / dataset - assert f.is_dir() - sdata = macsima(f, transformations=False) # Do not transform to make it easier to compare against pixel dimensions - el = sdata[list(sdata.images.keys())[0]] - cs = sdata.coordinate_systems[0] - - extent: dict[str, tuple[float, float]] = get_extent(el, coordinate_system=cs) - extent = {ax: (math.floor(extent[ax][0]), math.ceil(extent[ax][1])) for ax in extent} - assert extent == expected - - -@pytest.mark.parametrize( - "dataset,expected", - [("OMAP10_small", 4), ("OMAP23_small", 5)], -) -def test_total_channels(dataset: str, expected: int) -> None: - f = Path("./data") / dataset - assert f.is_dir() - sdata = macsima(f) - el = sdata[list(sdata.images.keys())[0]] - - # get the number of channels - channels: int = len(get_channel_names(el)) - assert channels == expected - - -@pytest.mark.parametrize( - "dataset,expected", - [ - ("OMAP10_small", ["R1 CD15", "R1 DAPI", "R2 Bcl 2", "R2 CD1c"]), - ( - "OMAP23_small", - ["R1 CD3", "R1 DAPI", "R2 CD279", "R4 CD66b", "R15 DAPI_background"], - ), - ], -) -def test_channel_names_with_cycle_in_name(dataset: str, expected: list[str]) -> None: - f = Path("./data") / dataset - assert f.is_dir() - sdata = macsima(f, include_cycle_in_channel_name=True) - el = sdata[list(sdata.images.keys())[0]] - - # get the channel names - channels = get_channel_names(el) - assert list(channels) == expected - - -@pytest.mark.parametrize( - "dataset,expected", - [ - ("OMAP10_small", 2), - ("OMAP23_small", 15), - ], -) -def test_total_rounds(dataset: str, expected: list[int]) -> None: - f = Path("./data") / dataset - assert f.is_dir() - sdata = macsima(f) - table = sdata[list(sdata.tables)[0]] - max_cycle = table.var["cycle"].max() - assert max_cycle == expected - - -@pytest.mark.parametrize( - "dataset,skip_rounds,expected", - [ - ("OMAP10_small", list(range(2, 4)), ["CD15", "DAPI"]), - ( - "OMAP23_small", - list(range(2, 16)), - ["CD3", "DAPI"], - ), - ], -) -def test_skip_rounds(dataset: str, skip_rounds: list[int], expected: list[str]) -> None: - f = Path("./data") / dataset - assert f.is_dir() - sdata = macsima(f, skip_rounds=skip_rounds) - el = sdata[list(sdata.images.keys())[0]] - - # get the channel names - channels = get_channel_names(el) - assert list(channels) == expected, f"Expected {expected}, got {list(channels)}" - - def test_unsupported_parsing_styles() -> None: with pytest.raises(ValueError, match="Invalid option `not_a_parsing_style` for `MACSimaParsingStyle`."): macsima(Path(), parsing_style="not_a_parsing_style") -def test_processed_single_folder_parsing_returns_a_single_image_stack(tmp_path: Path) -> None: - omap10_path = Path("./data/OMAP10_small") - shutil.copytree(omap10_path, tmp_path / "OMAP10_small_1") - shutil.copytree(omap10_path, tmp_path / "OMAP10_small_2") - - sdata = macsima(tmp_path, parsing_style="processed_single_folder") - - assert len(sdata.images) == 1 - # omap10_small has 4 channels, so we expect 8 here - el = sdata[list(sdata.images.keys())[0]] - assert len(get_channel_names(el)) == 8 - assert len(sdata.tables) == 1 - - -def test_processed_single_folder_parsing_warns_when_specifying_filtered_folders(tmp_path: Path) -> None: - omap10_path = Path("./data/OMAP10_small") - shutil.copytree(omap10_path, tmp_path / "OMAP10_small_1") - shutil.copytree(omap10_path, tmp_path / "OMAP10_small_2") - with pytest.warns(UserWarning, match="filtering only happens for processed_multi_folders"): - macsima(tmp_path, parsing_style="processed_single_folder", filter_folder_names=["OMAP10_small_2"]) - - -def test_processed_multiple_folders_returns_an_image_stack_per_subfolder(tmp_path: Path) -> None: - omap10_path = Path("./data/OMAP10_small") - shutil.copytree(omap10_path, tmp_path / "OMAP10_small_1") - shutil.copytree(omap10_path, tmp_path / "OMAP10_small_2") - - sdata = macsima(tmp_path, parsing_style="processed_multiple_folders") - - assert len(sdata.images) == 2 - for el in sdata.images.keys(): - assert len(get_channel_names(sdata[el])) == 4 - assert len(sdata.tables) == 2 - - -def test_processed_multiple_folders_skips_filtered_folder_names(tmp_path: Path) -> None: - shutil.copytree(Path("./data/OMAP10_small"), tmp_path / "OMAP10_small") - shutil.copytree(Path("./data/OMAP23_small"), tmp_path / "OMAP23_small") - - sdata = macsima(tmp_path, parsing_style="processed_multiple_folders", filter_folder_names=["OMAP10_small"]) - assert len(sdata.images) == 1 - assert list(sdata.images.keys()) == ["OMAP23_small_image"] - assert len(sdata.tables) == 1 - assert list(sdata.tables.keys()) == ["OMAP23_small_table"] - - -METADATA_COLUMN_ORDER = [ - "cycle", - "imagetype", - "well", - "ROI", - "fluorophore", - "clone", - "exposure", -] - -EXPECTED_METADATA_OMAP10 = pd.DataFrame( - { - "name": ["CD15", "DAPI", "Bcl 2", "CD1c"], - "cycle": [1, 1, 2, 2], - "imagetype": ["stain", "stain", "stain", "stain"], - "well": ["C-1", "C-1", "C-1", "C-1"], - "ROI": [1, 1, 1, 1], - "fluorophore": ["APC", "DAPI", "FITC", "PE"], - "clone": ["VIMC6", pd.NA, "REA872", "REA694"], - "exposure": [2304.0, 40.0, 96.0, 144.0], - }, - index=["CD15", "DAPI", "Bcl 2", "CD1c"], - columns=METADATA_COLUMN_ORDER, -) - -EXPECTED_METADATA_OMAP23 = pd.DataFrame( - { - "name": ["CD3", "DAPI", "CD279", "CD66b", "DAPI_background"], - "cycle": [1, 1, 2, 4, 15], - "imagetype": ["stain", "stain", "stain", "stain", "bleach"], - "well": ["D01", "D01", "D01", "D01", "D01"], - "ROI": [1, 1, 1, 1, 1], - "fluorophore": ["APC", "DAPI", "PE", "FITC", "DAPI"], - "clone": ["REA1151", pd.NA, "REA1165", "REA306", pd.NA], - "exposure": [1212.52, 51.0, 322.12, 856.68, 51.0], - }, - index=["CD3", "DAPI", "CD279", "CD66b", "DAPI_background"], - columns=METADATA_COLUMN_ORDER, -) - - -@pytest.mark.parametrize( - "dataset,expected_df", - [ - ("OMAP10_small", EXPECTED_METADATA_OMAP10), - ("OMAP23_small", EXPECTED_METADATA_OMAP23), - ], -) -def test_metadata_table(dataset: str, expected_df: pd.DataFrame) -> None: - f = Path("./data") / dataset - assert f.is_dir() - sdata = macsima(f) - table = sdata[list(sdata.tables.keys())[0]] - - # Convert table.var to a DataFrame and align to expected columns - actual = table.var[METADATA_COLUMN_ORDER] - - pd.testing.assert_frame_equal(actual, expected_df) - - def test_mci_sort_by_channel() -> None: sizes = [100, 200, 300] c_names = ["test11", "test3", "test2"] @@ -474,31 +225,6 @@ def test_mci_array_reference() -> None: assert da.all(mci.data[0] == orig_arr1) -@pytest.mark.parametrize("dataset", ["OMAP10_small", "OMAP23_small"]) -def test_cli_macsima(runner: CliRunner, dataset: str) -> None: - f = Path("./data") / dataset - assert f.is_dir() - with TemporaryDirectory() as tmpdir: - output_zarr = Path(tmpdir) / "data.zarr" - result = runner.invoke( - macsima_wrapper, - [ - "--input", - f, - "--output", - output_zarr, - "--subset", - 500, - "--c-subset", - 1, - "--multiscale", - False, - ], - ) - assert result.exit_code == 0, result.output - _ = read_zarr(output_zarr) - - def test_collect_map_annotation_values_with_no_duplicate_keys() -> None: ome = OME( structured_annotations=StructuredAnnotations( @@ -538,8 +264,6 @@ def test_collect_map_annotations_values_with_duplicate_keys_different_values() - ] ) ) - import re - result = _collect_map_annotation_values(ome) # The parser should return only the first found value. diff --git a/tests/readers/test_utils_image.py b/tests/unit/readers/utils/test_utils_image.py similarity index 100% rename from tests/readers/test_utils_image.py rename to tests/unit/readers/utils/test_utils_image.py diff --git a/tests/unit/readers/visium_hd/test_visium_hd.py b/tests/unit/readers/visium_hd/test_visium_hd.py new file mode 100644 index 00000000..677ab5d0 --- /dev/null +++ b/tests/unit/readers/visium_hd/test_visium_hd.py @@ -0,0 +1,32 @@ +import numpy as np + +from spatialdata_io.readers.visium_hd import ( + _decompose_projective_matrix, + _projective_matrix_is_affine, +) + +# --- UNIT TESTS FOR HELPER FUNCTIONS --- + + +def test_projective_matrix_is_affine() -> None: + """Test the affine matrix check function.""" + # An affine matrix should have [0, 0, 1] as its last row + affine_matrix = np.array([[2, 0.5, 10], [0.5, 2, 20], [0, 0, 1]]) + assert _projective_matrix_is_affine(affine_matrix) + + # A projective matrix is not affine if the last row is different + projective_matrix = np.array([[2, 0.5, 10], [0.5, 2, 20], [0.01, 0.02, 1]]) + assert not _projective_matrix_is_affine(projective_matrix) + + +def test_decompose_projective_matrix() -> None: + """Test the decomposition of a projective matrix into affine and shift components.""" + projective_matrix = np.array([[1, 2, 3], [4, 5, 6], [0.1, 0.2, 1]]) + affine, shift = _decompose_projective_matrix(projective_matrix) + + expected_affine = np.array([[1, 2, 3], [4, 5, 6], [0, 0, 1]]) + + # The affine component should be correctly extracted + assert np.allclose(affine, expected_affine) + # Recomposing the affine and shift matrices should yield the original projective matrix + assert np.allclose(affine @ shift, projective_matrix) diff --git a/tests/unit/readers/xenium/test_xenium.py b/tests/unit/readers/xenium/test_xenium.py new file mode 100644 index 00000000..943c63bd --- /dev/null +++ b/tests/unit/readers/xenium/test_xenium.py @@ -0,0 +1,105 @@ +from pathlib import Path + +import numpy as np +import pytest +from click.testing import CliRunner +from pytest_mock import MockerFixture + +from spatialdata_io.__main__ import xenium_wrapper +from spatialdata_io.readers.xenium import ( + _cell_id_str_from_prefix_suffix_uint32_reference, + cell_id_str_from_prefix_suffix_uint32, + prefix_suffix_uint32_from_cell_id_str, +) + + +def test_cell_id_str_from_prefix_suffix_uint32() -> None: + cell_id_prefix = np.array([1, 1437536272, 1437536273], dtype=np.uint32) + dataset_suffix = np.array([1, 1, 2]) + expected = np.array(["aaaaaaab-1", "ffkpbaba-1", "ffkpbabb-2"]) + + result = cell_id_str_from_prefix_suffix_uint32(cell_id_prefix, dataset_suffix) + reference = _cell_id_str_from_prefix_suffix_uint32_reference(cell_id_prefix, dataset_suffix) + assert np.array_equal(result, expected) + assert np.array_equal(reference, expected) + + +def test_cell_id_str_optimized_matches_reference() -> None: + rng = np.random.default_rng(42) + cell_id_prefix = rng.integers(0, 2**32, size=10_000, dtype=np.uint32) + dataset_suffix = rng.integers(0, 10, size=10_000) + + result = cell_id_str_from_prefix_suffix_uint32(cell_id_prefix, dataset_suffix) + reference = _cell_id_str_from_prefix_suffix_uint32_reference(cell_id_prefix, dataset_suffix) + assert np.array_equal(result, reference) + + +def test_prefix_suffix_uint32_from_cell_id_str() -> None: + cell_id_str = np.array(["aaaaaaab-1", "ffkpbaba-1", "ffkpbabb-2"]) + + cell_id_prefix, dataset_suffix = prefix_suffix_uint32_from_cell_id_str(cell_id_str) + assert np.array_equal(cell_id_prefix, np.array([1, 1437536272, 1437536273], dtype=np.uint32)) + assert np.array_equal(dataset_suffix, np.array([1, 1, 2])) + + +def test_roundtrip_with_data_limits() -> None: + # min and max values for uint32 + cell_id_prefix = np.array([0, 4294967295], dtype=np.uint32) + dataset_suffix = np.array([1, 1]) + cell_id_str = np.array(["aaaaaaaa-1", "pppppppp-1"]) + f0 = cell_id_str_from_prefix_suffix_uint32 + f1 = prefix_suffix_uint32_from_cell_id_str + assert np.array_equal(cell_id_prefix, f1(f0(cell_id_prefix, dataset_suffix))[0]) + assert np.array_equal(dataset_suffix, f1(f0(cell_id_prefix, dataset_suffix))[1]) + assert np.array_equal(cell_id_str, f0(*f1(cell_id_str))) + + +@pytest.mark.parametrize( + "kwarg_name", + ["--imread-kwargs", "--image-models-kwargs", "--labels-models-kwargs"], +) +def test_cli_xenium_invalid_json_rejected(runner: CliRunner, tmp_path: Path, kwarg_name: str) -> None: + """Invalid JSON for any kwargs option must produce a non-zero exit and a clear error.""" + result = runner.invoke( + xenium_wrapper, + [ + "--input", + str(tmp_path), + "--output", + str(tmp_path / "out.zarr"), + kwarg_name, + "not-valid-json{", + ], + ) + assert result.exit_code != 0 + assert "Invalid JSON" in result.output + + +@pytest.mark.parametrize( + ("kwarg_name", "kwarg_param"), + [ + ("--imread-kwargs", "imread_kwargs"), + ("--image-models-kwargs", "image_models_kwargs"), + ("--labels-models-kwargs", "labels_models_kwargs"), + ], +) +def test_cli_xenium_valid_json_forwarded( + runner: CliRunner, tmp_path: Path, mocker: MockerFixture, kwarg_name: str, kwarg_param: str +) -> None: + """Valid JSON kwargs must be parsed and forwarded to the xenium reader as a dict.""" + mock_xenium = mocker.patch("spatialdata_io.readers.xenium.xenium") + mock_xenium.return_value = mocker.MagicMock() + result = runner.invoke( + xenium_wrapper, + [ + "--input", + str(tmp_path), + "--output", + str(tmp_path / "out.zarr"), + kwarg_name, + '{"chunks": 512}', + ], + ) + assert result.exit_code == 0, result.output + call_kwargs = mock_xenium.call_args.kwargs + assert call_kwargs[kwarg_param] == {"chunks": 512} diff --git a/tests/unit/test_download_test_data.py b/tests/unit/test_download_test_data.py new file mode 100644 index 00000000..70b7c505 --- /dev/null +++ b/tests/unit/test_download_test_data.py @@ -0,0 +1,397 @@ +from __future__ import annotations + +import importlib.util +import io +import sys +import urllib.error +import urllib.request +import zipfile +from email.message import Message +from pathlib import Path +from typing import TYPE_CHECKING, Any + +import pytest + +if TYPE_CHECKING: + from types import ModuleType + + +SCRIPT_DIR = Path(__file__).parents[2] / "scripts" / "test_data_downloader" + + +def _load_module(module_name: str, module_path: Path) -> ModuleType: + spec = importlib.util.spec_from_file_location(module_name, module_path) + if spec is None or spec.loader is None: + raise RuntimeError(f"Could not import {module_path}") + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +manifest = _load_module("manifest", SCRIPT_DIR / "manifest.py") +download_test_data = _load_module("downloader", SCRIPT_DIR / "downloader.py") + + +def _make_dataset( + key: str = "example", + *, + group: str = "group", + url: str | None = None, + archive_name: str | None = None, + extracted_dir: str | None = None, + source: str = "example", + test_path: str = "", +) -> Any: + return download_test_data.TestDataset( + key=key, + group=group, + url=url or f"https://example.com/{key}.zip", + archive_name=archive_name or f"{key}.zip", + extracted_dir=extracted_dir or key, + source=source, + test_path=test_path, + ) + + +def _write_zip(path: Path) -> None: + with zipfile.ZipFile(path, "w") as archive: + archive.writestr("payload.txt", "ok") + + +class _BytesResponse(io.BytesIO): + def __enter__(self) -> _BytesResponse: + return self + + def __exit__(self, exc_type: object, exc: object, traceback: object) -> None: + self.close() + + +class TestDownload: + def test_uses_dataset_manifest_module(self) -> None: + assert download_test_data.TestDataset.__module__ == "manifest" + assert all(isinstance(dataset, manifest.TestDataset) for dataset in download_test_data.DATASETS) + + def test_sends_explicit_headers(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + seen_request: urllib.request.Request | None = None + + def urlopen(request: urllib.request.Request, timeout: int) -> _BytesResponse: + nonlocal seen_request + seen_request = request + assert timeout == download_test_data.DOWNLOAD_TIMEOUT_SEC + return _BytesResponse(b"payload") + + monkeypatch.setattr(download_test_data.urllib.request, "urlopen", urlopen) + + download_test_data._download(_make_dataset(), tmp_path / "archive.zip") + + assert seen_request is not None + assert seen_request.get_header("User-agent") == "curl/8.0.0" + assert seen_request.get_header("Accept") == "*/*" + + def test_reports_http_403_with_actionable_message(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + dataset = _make_dataset("forbidden") + + def urlopen(request: object, timeout: int) -> None: + raise urllib.error.HTTPError(dataset.url, 403, "Forbidden", hdrs=Message(), fp=None) + + monkeypatch.setattr(download_test_data.urllib.request, "urlopen", urlopen) + + with pytest.raises(download_test_data.DatasetDownloadError, match="HTTP 403 Forbidden") as exc_info: + download_test_data._download(dataset, tmp_path / dataset.archive_name) + + message = str(exc_info.value) + assert dataset.key in message + assert dataset.url in message + assert "rejected the request" in message + + def test_retries_transient_failures(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + dataset = _make_dataset("retry") + attempts = 0 + + def urlopen(request: object, timeout: int) -> _BytesResponse: + nonlocal attempts + attempts += 1 + if attempts == 1: + raise urllib.error.HTTPError(dataset.url, 503, "Service Unavailable", hdrs=Message(), fp=None) + return _BytesResponse(b"payload") + + monkeypatch.setattr(download_test_data.urllib.request, "urlopen", urlopen) + monkeypatch.setattr(download_test_data.time, "sleep", lambda seconds: None) + + download_test_data._download(dataset, tmp_path / dataset.archive_name) + + assert attempts == 2 + + +class TestDatasetManifest: + def test_manifest_is_valid(self) -> None: + manifest.validate_datasets() + + def test_returns_dataset_by_key(self) -> None: + dataset = manifest.get_dataset("seqfish") + + assert dataset.key == "seqfish" + assert dataset.extracted_dir == "seqfish-2-test-dataset" + + def test_reports_unknown_dataset_key(self) -> None: + with pytest.raises(KeyError, match="Unknown test dataset key 'missing'"): + manifest.get_dataset("missing") + + def test_test_path_defaults_to_extracted_directory_root(self) -> None: + dataset = manifest.get_dataset("xenium_breast") + + assert dataset.test_path == "" + + def test_can_filter_datasets_by_group(self) -> None: + datasets = manifest.datasets_by_group("macsima") + + assert {dataset.key for dataset in datasets} == {"macsima_omap10", "macsima_omap23"} + + def test_loads_datasets_from_toml(self, tmp_path: Path) -> None: + manifest_path = tmp_path / "datasets.toml" + manifest_path.write_text( + """ +[[datasets]] +key = "example" +group = "group" +url = "https://example.com/example.zip" +archive_name = "example.zip" +extracted_dir = "example" +source = "example source" +test_path = "nested" +""", + encoding="utf-8", + ) + + datasets = manifest.load_datasets(manifest_path) + + assert datasets == ( + manifest.TestDataset( + key="example", + group="group", + url="https://example.com/example.zip", + archive_name="example.zip", + extracted_dir="example", + source="example source", + test_path="nested", + ), + ) + + def test_rejects_duplicate_dataset_keys(self) -> None: + first = _make_dataset("duplicate", extracted_dir="first") + second = _make_dataset("duplicate", extracted_dir="second") + + with pytest.raises(ValueError, match="Duplicate test dataset key: 'duplicate'"): + manifest.validate_datasets((first, second)) + + def test_rejects_duplicate_extracted_dirs(self) -> None: + first = _make_dataset("first", extracted_dir="duplicate") + second = _make_dataset("second", extracted_dir="duplicate") + + with pytest.raises(ValueError, match="Duplicate test dataset extracted_dir: 'duplicate'"): + manifest.validate_datasets((first, second)) + + def test_rejects_empty_required_manifest_fields(self) -> None: + dataset = _make_dataset("missing-group", group="") + + with pytest.raises(ValueError, match="Dataset 'missing-group' has empty group"): + manifest.validate_datasets((dataset,)) + + def test_rejects_test_path_outside_extracted_dir(self) -> None: + dataset = _make_dataset("unsafe-test-path", test_path="../outside") + + with pytest.raises(ValueError, match="test_path must be a relative path"): + manifest.validate_datasets((dataset,)) + + def test_rejects_manifest_without_datasets_array(self, tmp_path: Path) -> None: + manifest_path = tmp_path / "datasets.toml" + manifest_path.write_text("datasets = { key = 'example' }\n", encoding="utf-8") + + with pytest.raises(ValueError, match=r"\[\[datasets\]\] array"): + manifest.load_datasets(manifest_path) + + def test_rejects_invalid_toml(self, tmp_path: Path) -> None: + manifest_path = tmp_path / "datasets.toml" + manifest_path.write_text("[[datasets]\n", encoding="utf-8") + + with pytest.raises(ValueError, match="Invalid dataset manifest TOML"): + manifest.load_datasets(manifest_path) + + def test_rejects_unknown_root_manifest_field(self, tmp_path: Path) -> None: + manifest_path = tmp_path / "datasets.toml" + manifest_path.write_text( + """ +version = 1 + +[[datasets]] +key = "example" +group = "group" +url = "https://example.com/example.zip" +archive_name = "example.zip" +extracted_dir = "example" +source = "example source" +""", + encoding="utf-8", + ) + + with pytest.raises(ValueError, match="unknown root field"): + manifest.load_datasets(manifest_path) + + def test_rejects_missing_required_manifest_field(self, tmp_path: Path) -> None: + manifest_path = tmp_path / "datasets.toml" + manifest_path.write_text( + """ +[[datasets]] +key = "example" +group = "group" +url = "https://example.com/example.zip" +archive_name = "example.zip" +extracted_dir = "example" +""", + encoding="utf-8", + ) + + with pytest.raises(ValueError, match="missing required field"): + manifest.load_datasets(manifest_path) + + def test_rejects_unknown_manifest_field(self, tmp_path: Path) -> None: + manifest_path = tmp_path / "datasets.toml" + manifest_path.write_text( + """ +[[datasets]] +key = "example" +group = "group" +url = "https://example.com/example.zip" +archive_name = "example.zip" +extracted_dir = "example" +source = "example source" +checksum = "unexpected" +""", + encoding="utf-8", + ) + + with pytest.raises(ValueError, match="unknown field"): + manifest.load_datasets(manifest_path) + + +class TestExtractZip: + def test_reports_invalid_archive(self, tmp_path: Path) -> None: + archive_path = tmp_path / "bad.zip" + archive_path.write_bytes(b"not a zip") + + with pytest.raises(download_test_data.DatasetDownloadError, match="not a valid zip archive"): + download_test_data._extract_zip(_make_dataset(), archive_path, tmp_path / "extract") + + def test_rejects_archive_members_outside_destination(self, tmp_path: Path) -> None: + archive_path = tmp_path / "unsafe.zip" + with zipfile.ZipFile(archive_path, "w") as archive: + archive.writestr("../escape.txt", "bad") + + with pytest.raises(download_test_data.DatasetDownloadError, match="outside"): + download_test_data._extract_zip(_make_dataset(), archive_path, tmp_path / "extract") + + assert not (tmp_path / "escape.txt").exists() + + +class TestDownloadDataset: + def test_skips_existing_dataset_without_force( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + dataset = _make_dataset("existing") + target = tmp_path / dataset.extracted_dir + target.mkdir() + + def fail_download(dataset: object, archive_path: Path) -> None: + raise AssertionError("download should have been skipped") + + monkeypatch.setattr(download_test_data, "_download", fail_download) + + download_test_data.download_dataset(dataset, tmp_path, force=False) + + assert "Skipping existing" in capsys.readouterr().out + + def test_replaces_existing_dataset_with_force(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + dataset = _make_dataset("force") + target = tmp_path / dataset.extracted_dir + target.mkdir() + (target / "old.txt").write_text("old", encoding="utf-8") + + def write_archive(dataset: object, archive_path: Path) -> None: + _write_zip(archive_path) + + monkeypatch.setattr(download_test_data, "_download", write_archive) + + download_test_data.download_dataset(dataset, tmp_path, force=True) + + assert not (target / "old.txt").exists() + assert (target / "payload.txt").read_text(encoding="utf-8") == "ok" + + def test_reports_archive_without_expected_contents(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + dataset = _make_dataset("empty") + + def write_empty_archive(dataset: object, archive_path: Path) -> None: + with zipfile.ZipFile(archive_path, "w"): + pass + + monkeypatch.setattr(download_test_data, "_download", write_empty_archive) + + with pytest.raises(download_test_data.DatasetDownloadError, match="expected directory"): + download_test_data.download_dataset(dataset, tmp_path, force=False) + + assert not (tmp_path / dataset.extracted_dir).exists() + + +class TestMain: + def test_downloads_multiple_selected_datasets(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + first = _make_dataset("first") + second = _make_dataset("second") + third = _make_dataset("third") + attempted: list[str] = [] + + def download_dataset(dataset: Any, output: Path, force: bool) -> None: + attempted.append(dataset.key) + assert output == tmp_path + assert not force + + monkeypatch.setattr(download_test_data, "DATASETS", (first, second, third)) + monkeypatch.setattr(download_test_data, "download_dataset", download_dataset) + + download_test_data.main(["--output", str(tmp_path), "--dataset", "first", "--dataset", "third"]) + + assert attempted == ["first", "third"] + + def test_lists_available_datasets( + self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + dataset = _make_dataset("listed", group="group", extracted_dir="listed-dir", source="listed source") + + monkeypatch.setattr(download_test_data, "DATASETS", (dataset,)) + + download_test_data.main(["--list"]) + + assert capsys.readouterr().out == "listed\tgroup\tlisted-dir\tlisted source\n" + + def test_continues_after_failure_then_exits_nonzero( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + first = _make_dataset("first") + second = _make_dataset("second") + attempted: list[str] = [] + + def download_dataset(dataset: Any, output: Path, force: bool) -> None: + attempted.append(dataset.key) + if dataset == first: + raise download_test_data.DatasetDownloadError(dataset, "download", "HTTP 403 Forbidden") + + monkeypatch.setattr(download_test_data, "DATASETS", (first, second)) + monkeypatch.setattr(download_test_data, "download_dataset", download_dataset) + + with pytest.raises(SystemExit) as exc_info: + download_test_data.main(["--output", str(tmp_path)]) + + assert exc_info.value.code == 1 + assert attempted == ["first", "second"] + output = capsys.readouterr().out + assert "Failed to download 1 dataset(s)" in output + assert "first" in output diff --git a/tests/test_init.py b/tests/unit/test_init.py similarity index 100% rename from tests/test_init.py rename to tests/unit/test_init.py