From 9a1ab6a8b5825ccf8d97c3264d0403aea34b1fb3 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Fri, 21 Aug 2026 11:10:53 +0200 Subject: [PATCH 1/5] chore: use scverse-misc for data downloading --- benchmarks/benchmarks/_utils.py | 2 + docs/release-notes/1.13.0a1.md | 2 +- docs/release-notes/4304.chore.md | 2 + pyproject.toml | 3 +- src/scanpy/_utils/__init__.py | 10 --- src/scanpy/datasets/_datasets.py | 47 +++-------- src/scanpy/datasets/_utils.py | 41 +++++++++- src/scanpy/datasets/registry.yaml | 55 +++++++++++++ src/scanpy/io/_download.py | 92 +++++---------------- src/scanpy/io/_read.py | 14 ++-- tests/test_datasets.py | 129 +++++++++++++++++++++--------- 11 files changed, 228 insertions(+), 169 deletions(-) create mode 100644 docs/release-notes/4304.chore.md create mode 100644 src/scanpy/datasets/registry.yaml diff --git a/benchmarks/benchmarks/_utils.py b/benchmarks/benchmarks/_utils.py index 3b7a7b9c7e..0feb397362 100644 --- a/benchmarks/benchmarks/_utils.py +++ b/benchmarks/benchmarks/_utils.py @@ -71,6 +71,7 @@ def _bmmc(n_obs: int = 4000) -> AnnData: registry = pooch.create( path=pooch.os_cache("pooch"), base_url="doi:10.6084/m9.figshare.22716739.v1/", + retry_if_failed=3, ) registry.load_registry_from_doi() samples = {smp: f"{smp}_filtered_feature_bc_matrix.h5" for smp in ("s1d1", "s1d3")} @@ -106,6 +107,7 @@ def _lung93k() -> AnnData: registry = pooch.create( path=pooch.os_cache("pooch"), base_url="doi:10.6084/m9.figshare.25664775.v1/", + retry_if_failed=3, ) registry.load_registry_from_doi() path = registry.fetch("adata.raw_compressed.h5ad") diff --git a/docs/release-notes/1.13.0a1.md b/docs/release-notes/1.13.0a1.md index 42563ba2de..5d19bf3fc7 100644 --- a/docs/release-notes/1.13.0a1.md +++ b/docs/release-notes/1.13.0a1.md @@ -18,7 +18,7 @@ - Add {func}`scanpy.pp.harmony_integrate` with Harmony1 and Harmony2 support for batch correction {smaller}`S Dicks, P Angerer` ({pr}`3953`) - Add support for {class}`numpy.random.Generator` to all functions previously accepting a `random_state` parameter {smaller}`P Angerer` ({pr}`3983`) - Add `layer` parameter to {func}`~scanpy.tl.filter_rank_genes_groups` {smaller}`P Angerer` ({pr}`3999`) -- add `sparse_format` parameter to `~scanpy.read_10x_mtx` defaulting to CSR {smaller}`E. Provo` ({pr}`4017`) +- add `sparse_format` parameter to {func}`~scanpy.io.read_10x_mtx` defaulting to CSR {smaller}`E. Provo` ({pr}`4017`) - Add `mean_in_log_space` argument to {func}`scanpy.tl.rank_genes_groups` for customizing how log-fold-change is calculated {user}`ilan-gold` ({pr}`4037`) - Introduce blazing-fast [`illico`][] as a new `wilcoxon` `method` in {func}`~scanpy.tl.rank_genes_groups`. Use it via either `method="wilcoxon_illico"` or via {attr}`~scanpy.Preset.ScanpyV2Preview` (preferred) {smaller}`I Gold` diff --git a/docs/release-notes/4304.chore.md b/docs/release-notes/4304.chore.md new file mode 100644 index 0000000000..c97cb2f7a2 --- /dev/null +++ b/docs/release-notes/4304.chore.md @@ -0,0 +1,2 @@ +Fetch the bundled datasets through {func}`scverse_misc.datasets.fetch`, adding hash verification, retries, and fall back to their pre-S3 upstream URLs. +{func}`~scanpy.io.read_10x_mtx` and friends now use `pooch` for downloading, also gaining retries {smaller}`P Angerer` diff --git a/pyproject.toml b/pyproject.toml index 32801799f5..0d605660f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,7 +52,6 @@ classifiers = [ dynamic = [ "version" ] dependencies = [ "anndata>=0.12.14", - "certifi", "fast-array-utils[accel,sparse]>=1.4", "h5py>=3.11", "joblib", @@ -67,7 +66,7 @@ dependencies = [ "pynndescent>=0.5.13", "scikit-learn>=1.6", "scipy>=1.15", - "scverse-misc[settings]>=0.1.1", + "scverse-misc[datasets,settings]>=0.1.3", "seaborn>=0.13.2", "session-info2", "statsmodels>=0.14.5", diff --git a/src/scanpy/_utils/__init__.py b/src/scanpy/_utils/__init__.py index 4ba294d12f..cd2ee92f52 100644 --- a/src/scanpy/_utils/__init__.py +++ b/src/scanpy/_utils/__init__.py @@ -37,7 +37,6 @@ if TYPE_CHECKING: from collections.abc import Callable, Iterable, KeysView, Mapping - from pathlib import Path from typing import Any from anndata import AnnData @@ -66,7 +65,6 @@ "axis_nnz", "check_array_function_arguments", "check_nonnegative_integers", - "check_presence_download", "check_use_raw", "compute_association_matrix_of_groups", "descend_classes_and_funcs", @@ -841,14 +839,6 @@ def select_groups( return groups_order_subset, groups_masks_obs -def check_presence_download(filename: Path, backup_url: str): - """Check if file is present otherwise download.""" - if not filename.is_file(): - from ..io._download import download - - download(backup_url, filename) - - # -------------------------------------------------------------------------------- # Neighbors # -------------------------------------------------------------------------------- diff --git a/src/scanpy/datasets/_datasets.py b/src/scanpy/datasets/_datasets.py index ca4004bacc..877bb781a2 100644 --- a/src/scanpy/datasets/_datasets.py +++ b/src/scanpy/datasets/_datasets.py @@ -18,8 +18,9 @@ from .._utils import _doc_params from .._utils._doctests import doctest_internet, doctest_needs, doctest_skipif from .._utils.random import _accepts_legacy_random_state, _legacy_random_state +from ..io._download import download from ..io._read import read, read_zarr -from ._utils import check_datasetdir_exists +from ._utils import check_datasetdir_exists, fetch_dataset if TYPE_CHECKING: from typing import Literal @@ -120,7 +121,6 @@ def blobs( @_doctest_skipif_old_anndata @doctest_internet -@check_datasetdir_exists def burczynski06() -> AnnData: """Bulk data with conditions ulcerative colitis (UC) and Crohn’s disease (CD) :cite:p:`Burczynski2006`. @@ -143,9 +143,7 @@ def burczynski06() -> AnnData: layers: None (.X) """ - filename = settings.datasetdir / "burczynski06/GDS1615_full.soft.gz" - url = "https://exampledata.scverse.org/scanpy/GDS1615_full.soft.gz" - return read(filename, backup_url=url) + return read(fetch_dataset("burczynski06")) @_doctest_skipif_old_anndata @@ -195,7 +193,6 @@ def krumsiek11() -> AnnData: @_doctest_skipif_old_anndata @doctest_internet @doctest_needs("openpyxl") -@check_datasetdir_exists def moignard15() -> AnnData: r"""Hematopoiesis in early mouse embryos :cite:p:`Moignard2015`. @@ -223,11 +220,7 @@ def moignard15() -> AnnData: layers: None (.X) """ - filename = settings.datasetdir / "moignard15/nbt.3154-S3.xlsx" - backup_url = ( - "https://exampledata.scverse.org/scanpy/41587_2015_BFnbt3154_MOESM4_ESM.xlsx" - ) - adata = read(filename, sheet="dCt_values.txt", backup_url=backup_url) + adata = read(fetch_dataset("moignard15"), sheet="dCt_values.txt") # filter out 4 genes as in Haghverdi et al. (2016) gene_subset = ~np.isin(adata.var_names, ["Eif2b1", "Mrpl19", "Polr2a", "Ubc"]) adata = adata[:, gene_subset].copy() # retain non-removed genes @@ -256,7 +249,6 @@ def moignard15() -> AnnData: @_doctest_skipif_old_anndata @doctest_internet -@check_datasetdir_exists def paul15() -> AnnData: """Development of Myeloid Progenitors :cite:p:`Paul2015`. @@ -281,11 +273,7 @@ def paul15() -> AnnData: """ import h5py - filename = settings.datasetdir / "paul15/paul15.h5" - filename.parent.mkdir(exist_ok=True) - backup_url = "https://exampledata.scverse.org/scanpy/paul15.h5" - _utils.check_presence_download(filename, backup_url) - with h5py.File(filename, "r") as f: + with h5py.File(fetch_dataset("paul15"), "r") as f: # Coercing to float32 for backwards compatibility x = f["data.debatched"][()].astype(np.float32) gene_names = f["data.debatched_rownames"][()].astype(str) @@ -429,7 +417,6 @@ def pbmc68k_reduced() -> AnnData: @_doctest_skipif_old_anndata @doctest_internet -@check_datasetdir_exists def pbmc3k() -> AnnData: r"""3k PBMCs from 10x Genomics. @@ -475,16 +462,13 @@ def pbmc3k() -> AnnData: layers: None (.X) """ - url = "https://exampledata.scverse.org/scanpy/pbmc3k_raw.h5ad" with warnings.catch_warnings(): warnings.filterwarnings("ignore", category=OldFormatWarning) - adata = read(settings.datasetdir / "pbmc3k_raw.h5ad", backup_url=url) - return adata + return read(fetch_dataset("pbmc3k")) @_doctest_skipif_old_anndata @doctest_internet -@check_datasetdir_exists def pbmc3k_processed() -> AnnData: """Processed 3k PBMCs from 10x Genomics. @@ -517,12 +501,10 @@ def pbmc3k_processed() -> AnnData: layers: None (.X) """ # noqa: D401 - url = "https://exampledata.scverse.org/scanpy/pbmc3k.h5ad" - with warnings.catch_warnings(): warnings.filterwarnings("ignore", category=OldFormatWarning) warnings.filterwarnings("ignore", r"Moving.*from.*uns.*to.*obsp", FutureWarning) - return read(settings.datasetdir / "pbmc3k_processed.h5ad", backup_url=url) + return read(fetch_dataset("pbmc3k_processed")) def _download_visium_dataset( @@ -556,9 +538,7 @@ def _download_visium_dataset( # Download spatial data tar_filename = f"{sample_id}_spatial.tar.gz" tar_pth = sample_dir / tar_filename - _utils.check_presence_download( - filename=tar_pth, backup_url=f"{url_prefix}/{tar_filename}" - ) + download(f"{url_prefix}/{tar_filename}", tar_pth) with tarfile.open(tar_pth) as f: f.extraction_filter = tarfile.data_filter for el in f: @@ -566,17 +546,14 @@ def _download_visium_dataset( f.extract(el, sample_dir) # Download counts - _utils.check_presence_download( - filename=sample_dir / "filtered_feature_bc_matrix.h5", - backup_url=f"{url_prefix}/{sample_id}_filtered_feature_bc_matrix.h5", + download( + f"{url_prefix}/{sample_id}_filtered_feature_bc_matrix.h5", + sample_dir / "filtered_feature_bc_matrix.h5", ) # Download image if download_image: - _utils.check_presence_download( - filename=sample_dir / "image.tif", - backup_url=f"{url_prefix}/{sample_id}_image.tif", - ) + download(f"{url_prefix}/{sample_id}_image.tif", sample_dir / "image.tif") return sample_dir diff --git a/src/scanpy/datasets/_utils.py b/src/scanpy/datasets/_utils.py index 1a77c9f326..494e2c831d 100644 --- a/src/scanpy/datasets/_utils.py +++ b/src/scanpy/datasets/_utils.py @@ -1,13 +1,21 @@ from __future__ import annotations -from functools import wraps +from dataclasses import replace +from functools import cache, wraps +from importlib.resources import as_file, files +from pathlib import Path from typing import TYPE_CHECKING +from scverse_misc.datasets import fetch, parse_registry, register_loader + +from .. import logging as logg from .._settings import settings if TYPE_CHECKING: from collections.abc import Callable + from scverse_misc.datasets import DatasetEntry, DownloadCB + def check_datasetdir_exists[**P, R](f: Callable[P, R]) -> Callable[P, R]: @wraps(f) @@ -16,3 +24,34 @@ def wrapper(*args: P.args, **kwargs: P.kwargs) -> R: return f(*args, **kwargs) return wrapper + + +@register_loader("scanpy") +def _load_file(entry: DatasetEntry, target: Path, download: DownloadCB, /) -> Path: + """Download the single file making up `entry` and return its path. + + Falls back to the entry’s ``fallback_urls`` (the pre-S3 upstream locations) + if our bucket is unreachable or serves something that fails the hash check. + """ + (file,) = entry.files + # `target` is `datasetdir/scanpy`; its parent keeps scanpy’s flat layout. + dest = target.parent + try: + return Path(download(file, dest=dest)) + except (OSError, ValueError) as e: + if (url := entry.metadata.get("fallback_urls", {}).get(file.name)) is None: + raise + logg.warning(f"Downloading {file.name} failed ({e}), falling back to {url}") + return Path(download(replace(file, url=url), dest=dest)) + + +@cache +def _registry() -> tuple[str | None, dict[str, DatasetEntry]]: + with as_file(files(__package__) / "registry.yaml") as path: + return parse_registry(path) + + +def fetch_dataset(name: str) -> Path: + """Download `name` into :attr:`~scanpy.settings.datasetdir` if needed, return its path.""" + base_url, datasets = _registry() + return fetch(datasets[name], settings.datasetdir, base_url=base_url) diff --git a/src/scanpy/datasets/registry.yaml b/src/scanpy/datasets/registry.yaml new file mode 100644 index 0000000000..fd98f265a1 --- /dev/null +++ b/src/scanpy/datasets/registry.yaml @@ -0,0 +1,55 @@ +# Datasets scanpy hosts itself, fetched via `scverse_misc.datasets.fetch`. +# +# Add a `sha256` for every file: it is what lets a truncated or corrupted +# download be detected (and retried) instead of surfacing as a parse error. +# +# `fallback_urls` are the upstream locations we used before our own S3 bucket +# (see #4011); they are used if the bucket is down or serves garbage. +base_url: https://exampledata.scverse.org/scanpy + +datasets: + + burczynski06: + type: scanpy + fallback_urls: + GDS1615_full.soft.gz: ftp://ftp.ncbi.nlm.nih.gov/geo/datasets/GDS1nnn/GDS1615/soft/GDS1615_full.soft.gz + files: + - name: GDS1615_full.soft.gz + s3_key: GDS1615_full.soft.gz + sha256: 86126c1a3c163ea20abb14c1a9711aaff34e6c492ef6dd86298bbaf18cc3f5f3 + + moignard15: + type: scanpy + fallback_urls: + nbt.3154-S3.xlsx: https://static-content.springer.com/esm/art%3A10.1038%2Fnbt.3154/MediaObjects/41587_2015_BFnbt3154_MOESM4_ESM.xlsx + files: + - name: nbt.3154-S3.xlsx + s3_key: 41587_2015_BFnbt3154_MOESM4_ESM.xlsx + sha256: e7b358fef2b6da115b7ee3ab4c7fc55fd80e210bba427902e4afc9118992747c + + paul15: + type: scanpy + fallback_urls: + paul15.h5: https://falexwolf.de/data/paul15.h5 + files: + - name: paul15.h5 + s3_key: paul15.h5 + sha256: 6161984f758dd464992edc23f1a8ab89b2081600130c6aa93a1ab12b3ada5bfb + + pbmc3k: + type: scanpy + fallback_urls: + pbmc3k_raw.h5ad: https://falexwolf.de/data/pbmc3k_raw.h5ad + files: + - name: pbmc3k_raw.h5ad + s3_key: pbmc3k_raw.h5ad + sha256: 89a96f1beaa2dd83a687666d3f19a4513ac27a2a2d12581fcd77afed7ea653a1 + + pbmc3k_processed: + type: scanpy + fallback_urls: + pbmc3k.h5ad: https://raw.githubusercontent.com/chanzuckerberg/cellxgene/main/example-dataset/pbmc3k.h5ad + files: + - name: pbmc3k.h5ad + s3_key: pbmc3k.h5ad + sha256: 0db367b991dd95809732b218539ede489bea99113807f62ebd7ccc970025fe38 diff --git a/src/scanpy/io/_download.py b/src/scanpy/io/_download.py index 07166627cf..768db2105e 100644 --- a/src/scanpy/io/_download.py +++ b/src/scanpy/io/_download.py @@ -2,11 +2,13 @@ from __future__ import annotations -from pathlib import Path, PurePath +from pathlib import PurePath +from typing import TYPE_CHECKING -from .. import logging as logg +if TYPE_CHECKING: + from pathlib import Path -__all__ = ["check_datafile_present_and_download", "download", "slugify"] +__all__ = ["download", "slugify"] def slugify(path: str | PurePath) -> str: @@ -25,72 +27,18 @@ def slugify(path: str | PurePath) -> str: def download(url: str, path: Path) -> None: - from ssl import create_default_context - from tempfile import NamedTemporaryFile - from urllib.request import Request, urlopen - - from certifi import contents - from tqdm.auto import tqdm - - blocksize = 1024 * 8 - blocknum = 0 - - # Write to a temp file and rename so readers never see a partial file (#4097). - tmp_path: Path | None = None - try: - req = Request(url, headers={"User-agent": "scanpy-user"}) - - with urlopen(req, context=create_default_context(cadata=contents())) as resp: - total = resp.info().get("content-length", None) - with ( - tqdm( - unit="B", - unit_scale=True, - miniters=1, - unit_divisor=1024, - total=total if total is None else int(total), - ) as t, - NamedTemporaryFile( - dir=path.parent, - prefix=f"{path.name}.", - suffix=".part", - delete=False, - ) as f, - ): - tmp_path = Path(f.name) - block = resp.read(blocksize) - while block: - f.write(block) - blocknum += 1 - t.update(len(block)) - block = resp.read(blocksize) - - tmp_path.replace(path) - tmp_path = None - - except (KeyboardInterrupt, Exception): - # Only remove our own temp file; leave path, which may be another process's. - if tmp_path is not None: - tmp_path.unlink(missing_ok=True) - raise - - -def check_datafile_present_and_download( - path: Path, backup_url: str | None = None -) -> bool: - """Check whether the file is present, otherwise download.""" - path = Path(path) - if path.is_file(): - return True - if backup_url is None: - return False - logg.info( - f"try downloading from url\n{backup_url}\n" - "... this may take a while but only happens once" - ) - if not path.parent.is_dir(): - logg.info(f"creating directory {path.parent}/ for saving data") - path.parent.mkdir(parents=True) - - download(backup_url, path) - return True + """Download `url` to `path`, unless it is already there. + + pooch writes to a temp file first, so `path` never holds a partial download + (#4097), and it retries a few times when the transfer fails. + """ + import pooch + + # A one-file `Pooch`: `pooch.retrieve` would do, but has no retry knob. + pooch.create( + path=path.parent, + base_url="", + registry={path.name: None}, + urls={path.name: url}, + retry_if_failed=3, + ).fetch(path.name, progressbar=True) diff --git a/src/scanpy/io/_read.py b/src/scanpy/io/_read.py index f7f9c20237..8ec0873324 100644 --- a/src/scanpy/io/_read.py +++ b/src/scanpy/io/_read.py @@ -29,7 +29,7 @@ from .. import logging as logg from .._compat import pkg_version from .._settings import AnnDataFileFormat, Default, settings -from ._download import check_datafile_present_and_download, slugify +from ._download import download, slugify if TYPE_CHECKING: from collections.abc import Callable @@ -202,9 +202,8 @@ def read_10x_h5( """ path = Path(filename) start = logg.info(f"reading {path}") - is_present = check_datafile_present_and_download(path, backup_url=backup_url) - if not is_present: - logg.debug(f"... did not find original file {path}") + if backup_url is not None: + download(backup_url, path) with h5py.File(str(path), "r") as f: v3 = "/matrix" in f if v3: @@ -679,9 +678,8 @@ def _read( # noqa: PLR0912, PLR0915 raise ValueError(msg) else: ext = is_valid_filename(filename, return_ext=True, ext=ext) - is_present = check_datafile_present_and_download(filename, backup_url=backup_url) - if not is_present: - logg.debug(f"... did not find original file {filename}") + if backup_url is not None: + download(backup_url, filename) # read hdf5 files if ext in {"h5", "h5ad"}: if sheet is None: @@ -702,7 +700,7 @@ def _read( # noqa: PLR0912, PLR0915 logg.info(f"... reading from cache file {path_cache}") return read_h5ad(path_cache) - if not is_present: + if not filename.is_file(): msg = f"Did not find file {filename}." raise FileNotFoundError(msg) logg.debug(f"reading {filename}") diff --git a/tests/test_datasets.py b/tests/test_datasets.py index 41f165b8c6..91282426e5 100644 --- a/tests/test_datasets.py +++ b/tests/test_datasets.py @@ -2,9 +2,7 @@ from __future__ import annotations -import io import subprocess -import urllib.request import warnings from collections import defaultdict from pathlib import Path @@ -12,7 +10,9 @@ from typing import TYPE_CHECKING import numpy as np +import pooch.core import pytest +import requests from anndata.tests.helpers import assert_adata_equal from packaging.version import Version @@ -24,9 +24,9 @@ if TYPE_CHECKING: from collections.abc import Callable - from typing import Self from anndata import AnnData + from pooch.typing import Downloader, Processor @pytest.fixture(autouse=True) @@ -164,6 +164,43 @@ def test_visium_datasets_images(): assert output == f"{image_path}: image/tiff" +def test_dataset_fallback_url(tmp_path: Path) -> None: + """A dead primary URL falls through to the registry’s `fallback_urls`.""" + from scverse_misc.datasets import DatasetEntry, FileEntry + + from scanpy.datasets._utils import _load_file + + entry = DatasetEntry( + name="fake", + type="scanpy", + files=(FileEntry(name="fake.bin", url="https://primary.invalid/fake.bin"),), + metadata=dict(fallback_urls={"fake.bin": "https://fallback.invalid/fake.bin"}), + ) + tried: list[str] = [] + + def download( + file: FileEntry, + /, + *, + dest: Path | None = None, + processor: Processor | None = None, + ) -> str: + tried.append(file.resolve_url()) + if len(tried) == 1: + msg = "bucket is down" + raise OSError(msg) + assert dest is not None + return str(dest / file.name) + + path = _load_file(entry, tmp_path / "scanpy", download) + + assert path == tmp_path / "fake.bin" + assert tried == [ + "https://primary.invalid/fake.bin", + "https://fallback.invalid/fake.bin", + ] + + def test_download_failure() -> None: from urllib.error import HTTPError @@ -172,33 +209,34 @@ def test_download_failure() -> None: excinfo.value.close() -def test_download_atomic(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.fixture +def fake_downloader(monkeypatch: pytest.MonkeyPatch) -> Callable[[Downloader], None]: + """Let a test stand in for the downloader pooch would pick for a URL.""" + + def use(downloader: Downloader) -> None: + monkeypatch.setattr( + pooch.core, "choose_downloader", lambda url, progressbar=False: downloader + ) + + return use + + +def test_download_atomic( + tmp_path: Path, fake_downloader: Callable[[Downloader], None] +) -> None: """The destination must not appear until the download finished (#4097).""" content = b"0123456789" * 5_000 dest = tmp_path / "cache" / "data.bin" dest.parent.mkdir() dest_present_during_download: list[bool] = [] - class FakeResponse: - def __init__(self) -> None: - self._buf = io.BytesIO(content) - - def info(self) -> dict[str, str]: - return {"content-length": str(len(content))} - - def read(self, size: int) -> bytes: - chunk = self._buf.read(size) - if chunk: + def downloader(url: str, output_file: str, pooch_: object) -> None: + with Path(output_file).open("wb") as f: + for start in range(0, len(content), 1024): dest_present_during_download.append(dest.exists()) - return chunk + f.write(content[start : start + 1024]) - def __enter__(self) -> Self: - return self - - def __exit__(self, *exc: object) -> bool: - return False - - monkeypatch.setattr(urllib.request, "urlopen", lambda *a, **k: FakeResponse()) + fake_downloader(downloader) download("http://example.invalid/data.bin", dest) @@ -208,35 +246,46 @@ def __exit__(self, *exc: object) -> bool: assert list(dest.parent.iterdir()) == [dest] -def test_download_failure_keeps_existing_file( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch +def test_download_leaves_nothing_behind_on_failure( + tmp_path: Path, fake_downloader: Callable[[Downloader], None] ) -> None: - """A failed download must not delete an already-present destination (#4097).""" + """A failed download must not leave a partial file behind (#4097).""" dest = tmp_path / "cache" / "data.bin" dest.parent.mkdir() - dest.write_bytes(b"complete") + attempts = 0 - class FailingResponse: - def info(self) -> dict[str, str]: - return {"content-length": "100"} + def downloader(url: str, output_file: str, pooch_: object) -> None: + nonlocal attempts + attempts += 1 + Path(output_file).write_bytes(b"partial") + msg = "connection reset" + raise requests.ConnectionError(msg) - def read(self, size: int) -> bytes: - msg = "connection reset" - raise OSError(msg) + fake_downloader(downloader) - def __enter__(self) -> Self: - return self + with pytest.raises(requests.ConnectionError, match="connection reset"): + download("http://example.invalid/data.bin", dest) - def __exit__(self, *exc: object) -> bool: - return False + assert attempts == 4 # the initial try plus `retry_if_failed=3` + assert list(dest.parent.iterdir()) == [] - monkeypatch.setattr(urllib.request, "urlopen", lambda *a, **k: FailingResponse()) - with pytest.raises(OSError, match="connection reset"): - download("http://example.invalid/data.bin", dest) +def test_download_keeps_existing_file( + tmp_path: Path, fake_downloader: Callable[[Downloader], None] +) -> None: + """An already-present destination is kept as-is, not re-downloaded (#4097).""" + dest = tmp_path / "cache" / "data.bin" + dest.parent.mkdir() + dest.write_bytes(b"complete") + + def downloader(url: str, output_file: str, pooch_: object) -> None: + pytest.fail("should not have been downloaded again") + + fake_downloader(downloader) + + download("http://example.invalid/data.bin", dest) assert dest.read_bytes() == b"complete" - assert list(dest.parent.iterdir()) == [dest] # These are tested via doctest From df9b5e5abdcb23b45d7e2145172a48466119e394 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Fri, 21 Aug 2026 11:13:41 +0200 Subject: [PATCH 2/5] fix name --- docs/release-notes/{4304.chore.md => 4310.chore.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/release-notes/{4304.chore.md => 4310.chore.md} (100%) diff --git a/docs/release-notes/4304.chore.md b/docs/release-notes/4310.chore.md similarity index 100% rename from docs/release-notes/4304.chore.md rename to docs/release-notes/4310.chore.md From 8db9363f7fdef932f75732253a9596c4df1e0abd Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Fri, 21 Aug 2026 11:27:13 +0200 Subject: [PATCH 3/5] rename files --- src/scanpy/datasets/registry.yaml | 12 ++++++------ tests/test_datasets.py | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/scanpy/datasets/registry.yaml b/src/scanpy/datasets/registry.yaml index fd98f265a1..18e78bb419 100644 --- a/src/scanpy/datasets/registry.yaml +++ b/src/scanpy/datasets/registry.yaml @@ -12,18 +12,18 @@ datasets: burczynski06: type: scanpy fallback_urls: - GDS1615_full.soft.gz: ftp://ftp.ncbi.nlm.nih.gov/geo/datasets/GDS1nnn/GDS1615/soft/GDS1615_full.soft.gz + burczynski06.soft.gz: ftp://ftp.ncbi.nlm.nih.gov/geo/datasets/GDS1nnn/GDS1615/soft/GDS1615_full.soft.gz files: - - name: GDS1615_full.soft.gz + - name: burczynski06.soft.gz s3_key: GDS1615_full.soft.gz sha256: 86126c1a3c163ea20abb14c1a9711aaff34e6c492ef6dd86298bbaf18cc3f5f3 moignard15: type: scanpy fallback_urls: - nbt.3154-S3.xlsx: https://static-content.springer.com/esm/art%3A10.1038%2Fnbt.3154/MediaObjects/41587_2015_BFnbt3154_MOESM4_ESM.xlsx + moignard15.xlsx: https://static-content.springer.com/esm/art%3A10.1038%2Fnbt.3154/MediaObjects/41587_2015_BFnbt3154_MOESM4_ESM.xlsx files: - - name: nbt.3154-S3.xlsx + - name: moignard15.xlsx s3_key: 41587_2015_BFnbt3154_MOESM4_ESM.xlsx sha256: e7b358fef2b6da115b7ee3ab4c7fc55fd80e210bba427902e4afc9118992747c @@ -48,8 +48,8 @@ datasets: pbmc3k_processed: type: scanpy fallback_urls: - pbmc3k.h5ad: https://raw.githubusercontent.com/chanzuckerberg/cellxgene/main/example-dataset/pbmc3k.h5ad + pbmc3k_processed.h5ad: https://raw.githubusercontent.com/chanzuckerberg/cellxgene/main/example-dataset/pbmc3k.h5ad files: - - name: pbmc3k.h5ad + - name: pbmc3k_processed.h5ad s3_key: pbmc3k.h5ad sha256: 0db367b991dd95809732b218539ede489bea99113807f62ebd7ccc970025fe38 diff --git a/tests/test_datasets.py b/tests/test_datasets.py index 91282426e5..713fc87b8c 100644 --- a/tests/test_datasets.py +++ b/tests/test_datasets.py @@ -286,6 +286,7 @@ def downloader(url: str, output_file: str, pooch_: object) -> None: download("http://example.invalid/data.bin", dest) assert dest.read_bytes() == b"complete" + assert list(dest.parent.iterdir()) == [dest] # These are tested via doctest From 35fd1a323e1945124f7ab34a9282828fcdbca00b Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Fri, 28 Aug 2026 11:43:47 +0200 Subject: [PATCH 4/5] upstream (cherry picked from commit 51cba7c02c845d14064bfe5bc88e278b0f64e0ee) --- pyproject.toml | 2 +- src/scanpy/datasets/_utils.py | 17 ++------------ src/scanpy/datasets/registry.yaml | 20 ++++++++-------- tests/test_datasets.py | 39 +------------------------------ 4 files changed, 14 insertions(+), 64 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0d605660f1..a5e0d845eb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,7 +66,7 @@ dependencies = [ "pynndescent>=0.5.13", "scikit-learn>=1.6", "scipy>=1.15", - "scverse-misc[datasets,settings]>=0.1.3", + "scverse-misc[datasets,settings]>=0.1.4", "seaborn>=0.13.2", "session-info2", "statsmodels>=0.14.5", diff --git a/src/scanpy/datasets/_utils.py b/src/scanpy/datasets/_utils.py index 494e2c831d..c85dda671e 100644 --- a/src/scanpy/datasets/_utils.py +++ b/src/scanpy/datasets/_utils.py @@ -1,6 +1,5 @@ from __future__ import annotations -from dataclasses import replace from functools import cache, wraps from importlib.resources import as_file, files from pathlib import Path @@ -8,7 +7,6 @@ from scverse_misc.datasets import fetch, parse_registry, register_loader -from .. import logging as logg from .._settings import settings if TYPE_CHECKING: @@ -28,21 +26,10 @@ def wrapper(*args: P.args, **kwargs: P.kwargs) -> R: @register_loader("scanpy") def _load_file(entry: DatasetEntry, target: Path, download: DownloadCB, /) -> Path: - """Download the single file making up `entry` and return its path. - - Falls back to the entry’s ``fallback_urls`` (the pre-S3 upstream locations) - if our bucket is unreachable or serves something that fails the hash check. - """ + """Download the single file making up `entry` and return its path.""" (file,) = entry.files # `target` is `datasetdir/scanpy`; its parent keeps scanpy’s flat layout. - dest = target.parent - try: - return Path(download(file, dest=dest)) - except (OSError, ValueError) as e: - if (url := entry.metadata.get("fallback_urls", {}).get(file.name)) is None: - raise - logg.warning(f"Downloading {file.name} failed ({e}), falling back to {url}") - return Path(download(replace(file, url=url), dest=dest)) + return Path(download(file, dest=target.parent)) @cache diff --git a/src/scanpy/datasets/registry.yaml b/src/scanpy/datasets/registry.yaml index 18e78bb419..9a44501472 100644 --- a/src/scanpy/datasets/registry.yaml +++ b/src/scanpy/datasets/registry.yaml @@ -11,45 +11,45 @@ datasets: burczynski06: type: scanpy - fallback_urls: - burczynski06.soft.gz: ftp://ftp.ncbi.nlm.nih.gov/geo/datasets/GDS1nnn/GDS1615/soft/GDS1615_full.soft.gz files: - name: burczynski06.soft.gz s3_key: GDS1615_full.soft.gz sha256: 86126c1a3c163ea20abb14c1a9711aaff34e6c492ef6dd86298bbaf18cc3f5f3 + fallback_urls: + - ftp://ftp.ncbi.nlm.nih.gov/geo/datasets/GDS1nnn/GDS1615/soft/GDS1615_full.soft.gz moignard15: type: scanpy - fallback_urls: - moignard15.xlsx: https://static-content.springer.com/esm/art%3A10.1038%2Fnbt.3154/MediaObjects/41587_2015_BFnbt3154_MOESM4_ESM.xlsx files: - name: moignard15.xlsx s3_key: 41587_2015_BFnbt3154_MOESM4_ESM.xlsx sha256: e7b358fef2b6da115b7ee3ab4c7fc55fd80e210bba427902e4afc9118992747c + fallback_urls: + - https://static-content.springer.com/esm/art%3A10.1038%2Fnbt.3154/MediaObjects/41587_2015_BFnbt3154_MOESM4_ESM.xlsx paul15: type: scanpy - fallback_urls: - paul15.h5: https://falexwolf.de/data/paul15.h5 files: - name: paul15.h5 s3_key: paul15.h5 sha256: 6161984f758dd464992edc23f1a8ab89b2081600130c6aa93a1ab12b3ada5bfb + fallback_urls: + - https://falexwolf.de/data/paul15.h5 pbmc3k: type: scanpy - fallback_urls: - pbmc3k_raw.h5ad: https://falexwolf.de/data/pbmc3k_raw.h5ad files: - name: pbmc3k_raw.h5ad s3_key: pbmc3k_raw.h5ad sha256: 89a96f1beaa2dd83a687666d3f19a4513ac27a2a2d12581fcd77afed7ea653a1 + fallback_urls: + - https://falexwolf.de/data/pbmc3k_raw.h5ad pbmc3k_processed: type: scanpy - fallback_urls: - pbmc3k_processed.h5ad: https://raw.githubusercontent.com/chanzuckerberg/cellxgene/main/example-dataset/pbmc3k.h5ad files: - name: pbmc3k_processed.h5ad s3_key: pbmc3k.h5ad sha256: 0db367b991dd95809732b218539ede489bea99113807f62ebd7ccc970025fe38 + fallback_urls: + - https://raw.githubusercontent.com/chanzuckerberg/cellxgene/main/example-dataset/pbmc3k.h5ad diff --git a/tests/test_datasets.py b/tests/test_datasets.py index 713fc87b8c..5a8f8360ff 100644 --- a/tests/test_datasets.py +++ b/tests/test_datasets.py @@ -26,7 +26,7 @@ from collections.abc import Callable from anndata import AnnData - from pooch.typing import Downloader, Processor + from pooch.typing import Downloader @pytest.fixture(autouse=True) @@ -164,43 +164,6 @@ def test_visium_datasets_images(): assert output == f"{image_path}: image/tiff" -def test_dataset_fallback_url(tmp_path: Path) -> None: - """A dead primary URL falls through to the registry’s `fallback_urls`.""" - from scverse_misc.datasets import DatasetEntry, FileEntry - - from scanpy.datasets._utils import _load_file - - entry = DatasetEntry( - name="fake", - type="scanpy", - files=(FileEntry(name="fake.bin", url="https://primary.invalid/fake.bin"),), - metadata=dict(fallback_urls={"fake.bin": "https://fallback.invalid/fake.bin"}), - ) - tried: list[str] = [] - - def download( - file: FileEntry, - /, - *, - dest: Path | None = None, - processor: Processor | None = None, - ) -> str: - tried.append(file.resolve_url()) - if len(tried) == 1: - msg = "bucket is down" - raise OSError(msg) - assert dest is not None - return str(dest / file.name) - - path = _load_file(entry, tmp_path / "scanpy", download) - - assert path == tmp_path / "fake.bin" - assert tried == [ - "https://primary.invalid/fake.bin", - "https://fallback.invalid/fake.bin", - ] - - def test_download_failure() -> None: from urllib.error import HTTPError From fd6cda7a91cd39b33f859fb735243f421f8306c9 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Fri, 28 Aug 2026 11:58:55 +0200 Subject: [PATCH 5/5] add intersphinx entry --- docs/conf.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/conf.py b/docs/conf.py index f7769e2635..f0e322babd 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -169,6 +169,7 @@ python=("https://docs.python.org/3", None), rapids_singlecell=("https://rapids-singlecell.readthedocs.io/en/latest/", None), scipy=("https://docs.scipy.org/doc/scipy/", None), + scverse_misc=("https://scverse-misc.readthedocs.io/stable/", None), seaborn=("https://seaborn.pydata.org/", None), session_info2=("https://session-info2.readthedocs.io/en/stable/", None), squidpy=("https://squidpy.readthedocs.io/en/stable/", None),