From bbfe0d42e720fe6fe7e13a5ae865728d5f43ba2d Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Fri, 4 Sep 2026 02:19:17 +0330 Subject: [PATCH 1/5] Warn when uploads omit unrecognized paths --- dandi/files/__init__.py | 88 +++++++++++++++++++++++++++++++++++++- dandi/tests/test_files.py | 53 +++++++++++++++++++++++ dandi/tests/test_upload.py | 65 ++++++++++++++++++++++++++++ dandi/upload.py | 27 ++++++++++++ 4 files changed, 232 insertions(+), 1 deletion(-) diff --git a/dandi/files/__init__.py b/dandi/files/__init__.py index 8b1d501bc..439614db9 100644 --- a/dandi/files/__init__.py +++ b/dandi/files/__init__.py @@ -12,7 +12,7 @@ from __future__ import annotations from collections import deque -from collections.abc import Iterator +from collections.abc import Iterable, Iterator import os.path from pathlib import Path @@ -66,10 +66,13 @@ "dandi_file", "find_dandi_files", "find_bids_dataset_description", + "find_unused_paths", ] lgr = get_logger() +_IGNORED_UPLOAD_PATH_NAMES = {"__MACOSX", "Thumbs.db"} + def find_dandi_files( *paths: str | Path, @@ -161,6 +164,89 @@ def find_dandi_files( yield df +def find_unused_paths( + paths: Iterable[str | Path], + used_paths: Iterable[str | Path], + *, + dandiset_path: str | Path, +) -> list[Path]: + """Find requested files and directories omitted by DANDI discovery. + + ``used_paths`` should contain the paths yielded by :func:`find_dandi_files`. + Unknown files are reported individually when a requested directory also + contains a recognized asset. If a requested directory contains no + recognized assets, the directory itself is reported once. Dot-prefixed + paths, the root ``dandiset.yaml`` file, empty directories, and symlinked + directories are treated as intentionally ignored. + """ + + root = Path(os.path.normcase(os.path.abspath(dandiset_path))) + + def normalize(path: str | Path) -> Path: + normalized = Path(os.path.normcase(os.path.abspath(path))) + try: + normalized.relative_to(root) + except ValueError: + raise ValueError( + f"Path {str(normalized)!r} is not inside Dandiset path {str(root)!r}" + ) from None + return normalized + + requested_paths = [normalize(path) for path in paths] + used = { + normalized + for path in used_paths + if (normalized := normalize(path)) != root / dandiset_metadata_file + } + + def is_ignored(path: Path) -> bool: + relative = path.relative_to(root) + return ( + any(part.startswith(".") for part in relative.parts) + or any(part in _IGNORED_UPLOAD_PATH_NAMES for part in relative.parts) + or path == root / dandiset_metadata_file + ) + + def scan(path: Path) -> tuple[list[Path], bool, bool]: + """Return omitted roots, recognized-path, and content flags.""" + + if path == root / dandiset_metadata_file: + return [], False, False + if path in used: + return [], True, True + if is_ignored(path): + return [], False, False + if path.is_symlink() and path.is_dir(): + return [], False, False + if not path.is_dir(): + if not path.exists() and not path.is_symlink(): + return [], False, False + return [path], False, True + + children = list(path.iterdir()) + omitted: list[Path] = [] + found_used = False + found_content = False + for child in children: + child_omitted, child_found_used, child_has_content = scan(child) + found_used |= child_found_used + found_content |= child_has_content + omitted.extend(child_omitted) + + if found_used: + return omitted, True, found_content + if found_content: + return [path], False, True + return [], False, False + + unused: set[Path] = set() + for path in requested_paths: + omitted, _found_used, _found_content = scan(path) + unused.update(omitted) + + return sorted(unused, key=lambda path: path.relative_to(root).as_posix()) + + def dandi_file( filepath: str | Path, dandiset_path: str | Path | None = None, diff --git a/dandi/tests/test_files.py b/dandi/tests/test_files.py index bc73aece2..6c6850799 100644 --- a/dandi/tests/test_files.py +++ b/dandi/tests/test_files.py @@ -30,6 +30,7 @@ ZarrBIDSAsset, dandi_file, find_dandi_files, + find_unused_paths, ) lgr = get_logger() @@ -185,6 +186,58 @@ def test_find_dandi_files(tmp_path: Path) -> None: ] +def test_find_unused_paths(tmp_path: Path) -> None: + (tmp_path / dandiset_metadata_file).touch() + (tmp_path / "known.nwb").touch() + (tmp_path / "unknown.txt").touch() + (tmp_path / "unknown-dir").mkdir() + (tmp_path / "unknown-dir" / "file.txt").touch() + (tmp_path / "mixed").mkdir() + (tmp_path / "mixed" / "known.nwb").touch() + (tmp_path / "mixed" / "sidecar.json").touch() + (tmp_path / "sample.zarr").mkdir() + (tmp_path / "sample.zarr" / "chunk").touch() + (tmp_path / "empty").mkdir() + (tmp_path / ".hidden").mkdir() + (tmp_path / ".hidden" / "secret.nwb").touch() + (tmp_path / "__MACOSX").mkdir() + (tmp_path / "__MACOSX" / "._known.nwb").touch() + (tmp_path / "Thumbs.db").touch() + + unused = find_unused_paths( + [tmp_path], + [ + tmp_path / dandiset_metadata_file, + tmp_path / "known.nwb", + tmp_path / "mixed" / "known.nwb", + tmp_path / "sample.zarr", + ], + dandiset_path=tmp_path, + ) + + assert [path.relative_to(tmp_path).as_posix() for path in unused] == [ + "mixed/sidecar.json", + "unknown-dir", + "unknown.txt", + ] + assert find_unused_paths( + [tmp_path / "mixed"], [tmp_path / "mixed" / "known.nwb"], dandiset_path=tmp_path + ) == [tmp_path / "mixed" / "sidecar.json"] + + +def test_find_unused_paths_ignores_symlinked_directory(tmp_path: Path) -> None: + target = tmp_path / "outside" + target.mkdir() + (target / "omitted.txt").touch() + symlink = tmp_path / "linked" + try: + symlink.symlink_to(target, target_is_directory=True) + except OSError as exc: + pytest.skip(f"cannot create directory symlink: {exc}") + + assert find_unused_paths([symlink], [], dandiset_path=tmp_path) == [] + + def test_find_dandi_files_with_bids(tmp_path: Path) -> None: mkpaths( tmp_path, diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index ccf6371e6..3c2cfb80b 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -314,6 +314,71 @@ def test_upload_bids_non_nwb_file(bids_dandiset: SampleDandiset) -> None: assert [asset.path for asset in bids_dandiset.dandiset.get_assets()] == ["README"] +def test_upload_warns_for_unrecognized_paths( + caplog: pytest.LogCaptureFixture, + new_dandiset: SampleDandiset, + simple2_nwb: Path, +) -> None: + copyfile(simple2_nwb, new_dandiset.dspath / "sub-01.nwb") + (new_dandiset.dspath / "sidecar.json").write_text("{}") + (new_dandiset.dspath / "notes").mkdir() + (new_dandiset.dspath / "notes" / "readme.txt").write_text("notes") + + with caplog.at_level("WARNING", logger="dandi"): + new_dandiset.upload() + + assert ( + "2 paths were not uploaded because they were not recognized as DANDI assets: " + "notes, sidecar.json" + ) in caplog.text + + +def test_upload_partial_does_not_warn_for_unrequested_paths( + caplog: pytest.LogCaptureFixture, + new_dandiset: SampleDandiset, + simple2_nwb: Path, +) -> None: + nwb_path = new_dandiset.dspath / "sub-01.nwb" + copyfile(simple2_nwb, nwb_path) + (new_dandiset.dspath / "sidecar.json").write_text("{}") + + with caplog.at_level("WARNING", logger="dandi"): + new_dandiset.upload(paths=[nwb_path]) + + assert "were not uploaded because they were not recognized" not in caplog.text + + +def test_upload_allow_any_path_suppresses_omission_warning( + caplog: pytest.LogCaptureFixture, new_dandiset: SampleDandiset +) -> None: + (new_dandiset.dspath / "notes.txt").write_text("notes") + + with caplog.at_level("WARNING", logger="dandi"): + new_dandiset.upload(allow_any_path=True) + + assert "were not uploaded because they were not recognized" not in caplog.text + + +def test_upload_omission_warning_survives_upload_error( + caplog: pytest.LogCaptureFixture, + mocker: MockerFixture, + new_dandiset: SampleDandiset, + simple2_nwb: Path, +) -> None: + copyfile(simple2_nwb, new_dandiset.dspath / "sub-01.nwb") + (new_dandiset.dspath / "sidecar.json").write_text("{}") + mocker.patch.object( + LocalFileAsset, "iter_upload", side_effect=UploadError("upload failed") + ) + + with caplog.at_level("WARNING", logger="dandi"), pytest.raises( + UploadError, match="upload failed" + ): + new_dandiset.upload() + + assert "1 path was not uploaded because it was not recognized" in caplog.text + + @sweep_embargo def test_upload_sync_zarr( mocker: MockerFixture, zarr_dandiset: SampleDandiset, embargo: bool diff --git a/dandi/upload.py b/dandi/upload.py index 6c13aca73..e1a973af9 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -45,6 +45,7 @@ LocalAsset, LocalDirectoryAsset, ZarrAsset, + find_unused_paths, ) from .misctypes import Digest from .support import pyout as pyouts @@ -245,6 +246,16 @@ def new_super_len(o: Any) -> int: ) lgr.info(f"Found {len(dandi_files)} files to consider") + omitted_paths = ( + [] + if allow_any_path + else find_unused_paths( + paths, + (dfile.filepath for dfile in dandi_files), + dandiset_path=dandiset.path, + ) + ) + # We will keep a shared set of "being processed" paths so # we could limit the number of them until # https://github.com/pyout/pyout/issues/87 @@ -461,8 +472,24 @@ def report_validation_failure() -> None: ) lgr.warning(msg) + def report_omitted_paths() -> None: + if not omitted_paths: + return + + relpaths = [ + path.relative_to(dandiset.path).as_posix() for path in omitted_paths + ] + lgr.warning( + "%s were not uploaded because they were not recognized as DANDI " + "assets: %s. Review the paths or use --allow-any-path if intentional.", + pluralize(len(relpaths), "path"), + ", ".join(relpaths[:10]) + (", ..." if len(relpaths) > 10 else ""), + ) + lgr.debug("Complete list of paths not uploaded: %s", ", ".join(relpaths)) + with ExitStack() as warning_stack, out: warning_stack.callback(report_validation_failure) + warning_stack.callback(report_omitted_paths) for dfile in dandi_files: while len(process_paths) >= 10: lgr.log(2, "Sleep waiting for some paths to finish processing") From 028cf0684307d55a54c728f7e8c3f346d77b701a Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Sat, 5 Sep 2026 22:17:44 +0330 Subject: [PATCH 2/5] test: use valid subject layout for upload warnings --- dandi/tests/test_upload.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index 3c2cfb80b..11cc2aa3c 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -319,7 +319,9 @@ def test_upload_warns_for_unrecognized_paths( new_dandiset: SampleDandiset, simple2_nwb: Path, ) -> None: - copyfile(simple2_nwb, new_dandiset.dspath / "sub-01.nwb") + subject_dir = new_dandiset.dspath / "sub-01" + subject_dir.mkdir() + copyfile(simple2_nwb, subject_dir / "sub-01.nwb") (new_dandiset.dspath / "sidecar.json").write_text("{}") (new_dandiset.dspath / "notes").mkdir() (new_dandiset.dspath / "notes" / "readme.txt").write_text("notes") @@ -338,7 +340,9 @@ def test_upload_partial_does_not_warn_for_unrequested_paths( new_dandiset: SampleDandiset, simple2_nwb: Path, ) -> None: - nwb_path = new_dandiset.dspath / "sub-01.nwb" + subject_dir = new_dandiset.dspath / "sub-01" + subject_dir.mkdir() + nwb_path = subject_dir / "sub-01.nwb" copyfile(simple2_nwb, nwb_path) (new_dandiset.dspath / "sidecar.json").write_text("{}") @@ -365,7 +369,9 @@ def test_upload_omission_warning_survives_upload_error( new_dandiset: SampleDandiset, simple2_nwb: Path, ) -> None: - copyfile(simple2_nwb, new_dandiset.dspath / "sub-01.nwb") + subject_dir = new_dandiset.dspath / "sub-01" + subject_dir.mkdir() + copyfile(simple2_nwb, subject_dir / "sub-01.nwb") (new_dandiset.dspath / "sidecar.json").write_text("{}") mocker.patch.object( LocalFileAsset, "iter_upload", side_effect=UploadError("upload failed") From 8ce3d3a1ad03e598be290201d11af0e69e664106 Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Sat, 5 Sep 2026 22:40:43 +0330 Subject: [PATCH 3/5] fix: use singular warning grammar --- dandi/upload.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dandi/upload.py b/dandi/upload.py index e1a973af9..f57e7edbf 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -479,10 +479,12 @@ def report_omitted_paths() -> None: relpaths = [ path.relative_to(dandiset.path).as_posix() for path in omitted_paths ] + verb = "was" if len(relpaths) == 1 else "were" lgr.warning( - "%s were not uploaded because they were not recognized as DANDI " + "%s %s not uploaded because they were not recognized as DANDI " "assets: %s. Review the paths or use --allow-any-path if intentional.", pluralize(len(relpaths), "path"), + verb, ", ".join(relpaths[:10]) + (", ..." if len(relpaths) > 10 else ""), ) lgr.debug("Complete list of paths not uploaded: %s", ", ".join(relpaths)) From 9f772b605f6957cbe686e68a56bb551b86d32eab Mon Sep 17 00:00:00 2001 From: Amirali Moradniaei Date: Wed, 9 Sep 2026 21:14:25 +0330 Subject: [PATCH 4/5] test: cover omitted upload path discovery edges --- dandi/tests/test_files.py | 1428 +++++++++++++++++++------------------ 1 file changed, 727 insertions(+), 701 deletions(-) diff --git a/dandi/tests/test_files.py b/dandi/tests/test_files.py index 6c6850799..28ce7841a 100644 --- a/dandi/tests/test_files.py +++ b/dandi/tests/test_files.py @@ -1,701 +1,727 @@ -from __future__ import annotations - -from operator import attrgetter -import os -from pathlib import Path -import subprocess -from unittest.mock import ANY - -from dandischema.models import get_schema_version -import numpy as np -import pytest -import zarr - -from .fixtures import SampleDandiset -from .test_helpers import TWO_ARRAY_ZARR_LAYOUT, zarr_format_of -from .. import get_logger -from ..consts import ZARR_MIME_TYPE, dandiset_metadata_file -from ..dandiapi import AssetType, RemoteZarrAsset -from ..exceptions import UnknownAssetError -from ..files import ( - BIDSDatasetDescriptionAsset, - DandisetMetadataFile, - GenericAsset, - GenericBIDSAsset, - ImageAsset, - NWBAsset, - NWBBIDSAsset, - VideoAsset, - ZarrAsset, - ZarrBIDSAsset, - dandi_file, - find_dandi_files, - find_unused_paths, -) - -lgr = get_logger() - - -def mkpaths(root: Path, *paths: str) -> None: - for p in paths: - pp = root / p - pp.parent.mkdir(parents=True, exist_ok=True) - if p.endswith("/"): - pp.mkdir() - else: - pp.touch() - - -def test_find_dandi_files(tmp_path: Path) -> None: - mkpaths( - tmp_path, - dandiset_metadata_file, - "sample01.zarr/inner.nwb", - "sample01.zarr/foo", - "sample02.nwb", - "foo", - "bar.txt", - "subdir/sample03.nwb", - "subdir/sample04.zarr/inner2.nwb", - "subdir/sample04.zarr/baz", - "subdir/gnusto", - "subdir/cleesh.txt", - "empty.zarr/", - "glarch.mp4", - "quux.png", - ".ignored", - ".ignored.dir/ignored.nwb", - ) - - files = sorted( - find_dandi_files(tmp_path, dandiset_path=tmp_path), key=attrgetter("filepath") - ) - assert files == [ - VideoAsset( - filepath=tmp_path / "glarch.mp4", path="glarch.mp4", dandiset_path=tmp_path - ), - ImageAsset( - filepath=tmp_path / "quux.png", path="quux.png", dandiset_path=tmp_path - ), - ZarrAsset( - filepath=tmp_path / "sample01.zarr", - path="sample01.zarr", - dandiset_path=tmp_path, - ), - NWBAsset( - filepath=tmp_path / "sample02.nwb", - path="sample02.nwb", - dandiset_path=tmp_path, - ), - NWBAsset( - filepath=tmp_path / "subdir" / "sample03.nwb", - path="subdir/sample03.nwb", - dandiset_path=tmp_path, - ), - ZarrAsset( - filepath=tmp_path / "subdir" / "sample04.zarr", - path="subdir/sample04.zarr", - dandiset_path=tmp_path, - ), - ] - - files = sorted( - find_dandi_files(tmp_path, dandiset_path=tmp_path, allow_all=True), - key=attrgetter("filepath"), - ) - assert files == [ - GenericAsset( - filepath=tmp_path / "bar.txt", path="bar.txt", dandiset_path=tmp_path - ), - DandisetMetadataFile( - filepath=tmp_path / dandiset_metadata_file, dandiset_path=tmp_path - ), - GenericAsset(filepath=tmp_path / "foo", path="foo", dandiset_path=tmp_path), - VideoAsset( - filepath=tmp_path / "glarch.mp4", path="glarch.mp4", dandiset_path=tmp_path - ), - ImageAsset( - filepath=tmp_path / "quux.png", path="quux.png", dandiset_path=tmp_path - ), - ZarrAsset( - filepath=tmp_path / "sample01.zarr", - path="sample01.zarr", - dandiset_path=tmp_path, - ), - NWBAsset( - filepath=tmp_path / "sample02.nwb", - path="sample02.nwb", - dandiset_path=tmp_path, - ), - GenericAsset( - filepath=tmp_path / "subdir" / "cleesh.txt", - path="subdir/cleesh.txt", - dandiset_path=tmp_path, - ), - GenericAsset( - filepath=tmp_path / "subdir" / "gnusto", - path="subdir/gnusto", - dandiset_path=tmp_path, - ), - NWBAsset( - filepath=tmp_path / "subdir" / "sample03.nwb", - path="subdir/sample03.nwb", - dandiset_path=tmp_path, - ), - ZarrAsset( - filepath=tmp_path / "subdir" / "sample04.zarr", - path="subdir/sample04.zarr", - dandiset_path=tmp_path, - ), - ] - - files = sorted( - find_dandi_files(tmp_path, dandiset_path=tmp_path, include_metadata=True), - key=attrgetter("filepath"), - ) - assert files == [ - DandisetMetadataFile( - filepath=tmp_path / dandiset_metadata_file, dandiset_path=tmp_path - ), - VideoAsset( - filepath=tmp_path / "glarch.mp4", path="glarch.mp4", dandiset_path=tmp_path - ), - ImageAsset( - filepath=tmp_path / "quux.png", path="quux.png", dandiset_path=tmp_path - ), - ZarrAsset( - filepath=tmp_path / "sample01.zarr", - path="sample01.zarr", - dandiset_path=tmp_path, - ), - NWBAsset( - filepath=tmp_path / "sample02.nwb", - path="sample02.nwb", - dandiset_path=tmp_path, - ), - NWBAsset( - filepath=tmp_path / "subdir" / "sample03.nwb", - path="subdir/sample03.nwb", - dandiset_path=tmp_path, - ), - ZarrAsset( - filepath=tmp_path / "subdir" / "sample04.zarr", - path="subdir/sample04.zarr", - dandiset_path=tmp_path, - ), - ] - - -def test_find_unused_paths(tmp_path: Path) -> None: - (tmp_path / dandiset_metadata_file).touch() - (tmp_path / "known.nwb").touch() - (tmp_path / "unknown.txt").touch() - (tmp_path / "unknown-dir").mkdir() - (tmp_path / "unknown-dir" / "file.txt").touch() - (tmp_path / "mixed").mkdir() - (tmp_path / "mixed" / "known.nwb").touch() - (tmp_path / "mixed" / "sidecar.json").touch() - (tmp_path / "sample.zarr").mkdir() - (tmp_path / "sample.zarr" / "chunk").touch() - (tmp_path / "empty").mkdir() - (tmp_path / ".hidden").mkdir() - (tmp_path / ".hidden" / "secret.nwb").touch() - (tmp_path / "__MACOSX").mkdir() - (tmp_path / "__MACOSX" / "._known.nwb").touch() - (tmp_path / "Thumbs.db").touch() - - unused = find_unused_paths( - [tmp_path], - [ - tmp_path / dandiset_metadata_file, - tmp_path / "known.nwb", - tmp_path / "mixed" / "known.nwb", - tmp_path / "sample.zarr", - ], - dandiset_path=tmp_path, - ) - - assert [path.relative_to(tmp_path).as_posix() for path in unused] == [ - "mixed/sidecar.json", - "unknown-dir", - "unknown.txt", - ] - assert find_unused_paths( - [tmp_path / "mixed"], [tmp_path / "mixed" / "known.nwb"], dandiset_path=tmp_path - ) == [tmp_path / "mixed" / "sidecar.json"] - - -def test_find_unused_paths_ignores_symlinked_directory(tmp_path: Path) -> None: - target = tmp_path / "outside" - target.mkdir() - (target / "omitted.txt").touch() - symlink = tmp_path / "linked" - try: - symlink.symlink_to(target, target_is_directory=True) - except OSError as exc: - pytest.skip(f"cannot create directory symlink: {exc}") - - assert find_unused_paths([symlink], [], dandiset_path=tmp_path) == [] - - -def test_find_dandi_files_with_bids(tmp_path: Path) -> None: - mkpaths( - tmp_path, - dandiset_metadata_file, - "foo.txt", - "bar.nwb", - "bids1/.bidsignore", - "bids1/dataset_description.json", - "bids1/file.txt", - "bids1/subdir/quux.nwb", - "bids1/subdir/glarch.zarr/dataset_description.json", - "bids2/dataset_description.json", - "bids2/movie.mp4", - "bids2/subbids/dataset_description.json", - "bids2/subbids/data.json", - ) - - files = sorted( - find_dandi_files(tmp_path, dandiset_path=tmp_path, allow_all=False), - key=attrgetter("filepath"), - ) - - assert files == [ - NWBAsset(filepath=tmp_path / "bar.nwb", path="bar.nwb", dandiset_path=tmp_path), - GenericBIDSAsset( - filepath=tmp_path / "bids1" / ".bidsignore", - path="bids1/.bidsignore", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - BIDSDatasetDescriptionAsset( - filepath=tmp_path / "bids1" / "dataset_description.json", - path="bids1/dataset_description.json", - dandiset_path=tmp_path, - dataset_files=ANY, # type: ignore[arg-type] - ), - GenericBIDSAsset( - filepath=tmp_path / "bids1" / "file.txt", - path="bids1/file.txt", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - ZarrBIDSAsset( - filepath=tmp_path / "bids1" / "subdir" / "glarch.zarr", - path="bids1/subdir/glarch.zarr", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - NWBBIDSAsset( - filepath=tmp_path / "bids1" / "subdir" / "quux.nwb", - path="bids1/subdir/quux.nwb", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - BIDSDatasetDescriptionAsset( - filepath=tmp_path / "bids2" / "dataset_description.json", - path="bids2/dataset_description.json", - dandiset_path=tmp_path, - dataset_files=ANY, # type: ignore[arg-type] - ), - GenericBIDSAsset( - filepath=tmp_path / "bids2" / "movie.mp4", - path="bids2/movie.mp4", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - GenericBIDSAsset( - filepath=tmp_path / "bids2" / "subbids" / "data.json", - path="bids2/subbids/data.json", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - GenericBIDSAsset( - filepath=tmp_path / "bids2" / "subbids" / "dataset_description.json", - path="bids2/subbids/dataset_description.json", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - ] - - bidsdd = files[2] - assert isinstance(bidsdd, BIDSDatasetDescriptionAsset) - assert sorted(bidsdd.dataset_files, key=attrgetter("filepath")) == [ - GenericBIDSAsset( - filepath=tmp_path / "bids1" / ".bidsignore", - path="bids1/.bidsignore", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - GenericBIDSAsset( - filepath=tmp_path / "bids1" / "file.txt", - path="bids1/file.txt", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - ZarrBIDSAsset( - filepath=tmp_path / "bids1" / "subdir" / "glarch.zarr", - path="bids1/subdir/glarch.zarr", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - NWBBIDSAsset( - filepath=tmp_path / "bids1" / "subdir" / "quux.nwb", - path="bids1/subdir/quux.nwb", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - ] - for asset in bidsdd.dataset_files: - assert asset.bids_dataset_description is bidsdd - - bidsdd = files[6] - assert isinstance(bidsdd, BIDSDatasetDescriptionAsset) - assert sorted(bidsdd.dataset_files, key=attrgetter("filepath")) == [ - GenericBIDSAsset( - filepath=tmp_path / "bids2" / "movie.mp4", - path="bids2/movie.mp4", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - GenericBIDSAsset( - filepath=tmp_path / "bids2" / "subbids" / "data.json", - path="bids2/subbids/data.json", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - GenericBIDSAsset( - filepath=tmp_path / "bids2" / "subbids" / "dataset_description.json", - path="bids2/subbids/dataset_description.json", - dandiset_path=tmp_path, - bids_dataset_description_ref=ANY, # type: ignore[arg-type] - ), - ] - for asset in bidsdd.dataset_files: - assert asset.bids_dataset_description is bidsdd - - -# This test sometimes fails and sometimes passes when running on NFS. -@pytest.mark.flaky(reruns=10) -def test_dandi_file_zarr_with_excluded_dotfiles(tmp_path: Path) -> None: - zarr_path = tmp_path / "foo.zarr" - mkpaths( - zarr_path, - ".git/data", - ".gitattributes", - ".dandi/somefile.txt", - ".datalad/", - "arr_0/.gitmodules", - ) - with pytest.raises(UnknownAssetError): - dandi_file(zarr_path) - with (zarr_path / "arr_0" / "foo").open("w") as fp: - print("Text.", file=fp) - # Force changes to be synced when testing on NFS: - fp.flush() - os.fsync(fp.fileno()) - zf = dandi_file(zarr_path) - assert isinstance(zf, ZarrAsset) - - -def test_validate_simple1(simple1_nwb: Path) -> None: - # this file should be ok as long as schema_version is specified - errors = dandi_file(simple1_nwb).get_validation_errors( - schema_version=get_schema_version() - ) - assert errors == [] - - -def test_validate_simple1_no_subject(simple1_nwb: Path) -> None: - errors = dandi_file(simple1_nwb).get_validation_errors() - errmsgs = [] - for e in errors: - assert e.message is not None - errmsgs.append(e.message) - assert errmsgs == ["Subject is missing."] - - -def test_validate_simple2(organized_nwb_dir: Path) -> None: - # this file should be ok since a Subject is included - errors = dandi_file( - organized_nwb_dir / "sub-mouse001" / "sub-mouse001.nwb", - dandiset_path=organized_nwb_dir, - ).get_validation_errors() - assert not errors - - -def test_validate_simple2_new(organized_nwb_dir: Path) -> None: - # this file should be ok - errors = dandi_file( - organized_nwb_dir / "sub-mouse001" / "sub-mouse001.nwb", - dandiset_path=organized_nwb_dir, - ).get_validation_errors(schema_version=get_schema_version()) - assert not errors - - -def test_validate_simple3_no_subject_id(simple3_nwb: Path) -> None: - errors = dandi_file(simple3_nwb).get_validation_errors() - errmsgs = [] - for e in errors: - assert e.message is not None - errmsgs.append(e.message) - assert errmsgs == ["subject_id is missing."] - - -def test_validate_bogus(tmp_path): - """ - Notes - ----- - * Intended to produce use-case for https://github.com/dandi/dandi-cli/issues/93 - but it would be tricky, so it is more of a smoke test that - we do not crash - """ - path = tmp_path / "wannabe.nwb" - path.write_text("not really nwb") - errors = dandi_file(path).get_validation_errors() - # ATM we would get 2 errors -- since could not be open in two places, - # but that would be too rigid to test. Let's just see that we have expected errors - assert any( - e.message.startswith( - ( - "Unable to open file", - "Unable to synchronously open file", - "Could not find an IO to read the file", - ) - ) - for e in errors - ) - # Recent versions of hdf5 changed the error message, hence the need to - # check for two different patterns. - - -def test_upload_zarr(new_dandiset, tmp_path): - filepath = tmp_path / "example.zarr" - zarr.save(filepath, np.arange(1000), np.arange(1000, 0, -1)) - layout = TWO_ARRAY_ZARR_LAYOUT[zarr_format_of(filepath)] - root_meta = layout["root_meta"] - zf = dandi_file(filepath) - assert isinstance(zf, ZarrAsset) - asset = zf.upload(new_dandiset.dandiset, {"description": "A test Zarr"}) - assert isinstance(asset, RemoteZarrAsset) - assert asset.asset_type is AssetType.ZARR - assert asset.path == "example.zarr" - md = asset.get_raw_metadata() - assert md["encodingFormat"] == ZARR_MIME_TYPE - assert md["description"] == "A test Zarr" - md["description"] = "A modified Zarr" - asset.set_raw_metadata(md) - md = asset.get_raw_metadata() - assert md["description"] == "A modified Zarr" - - entries = sorted(asset.iterfiles(), key=attrgetter("parts")) - assert [str(e) for e in entries] == layout["files"] - - entries = sorted(zf.iterfiles(include_dirs=True), key=attrgetter("parts")) - assert [str(e) for e in entries] == layout["files_and_dirs"] - # The root group metadata file is ``.zgroup`` in V2 and ``zarr.json`` in - # V3; either way it must be a real file at the Zarr root. - assert (zf.filetree / root_meta).exists() - assert (zf.filetree / root_meta).is_file() - assert not (zf.filetree / root_meta).is_dir() - assert (zf.filetree / "arr_0").exists() - assert not (zf.filetree / "arr_0").is_file() - assert (zf.filetree / "arr_0").is_dir() - assert not (zf.filetree / "0").exists() - assert not (zf.filetree / "0").is_file() - assert not (zf.filetree / "0").is_dir() - # ``arr_0/.zgroup`` never exists: in V2 ``arr_0`` is an array (uses - # ``.zarray``); in V3 ``.zgroup`` is not used at all. - assert not (zf.filetree / "arr_0" / ".zgroup").exists() - assert not (zf.filetree / "arr_0" / ".zgroup").is_file() - assert not (zf.filetree / "arr_0" / ".zgroup").is_dir() - assert not (zf.filetree / ".zgroup" / "0").exists() - assert not (zf.filetree / ".zgroup" / "0").is_file() - assert not (zf.filetree / ".zgroup" / "0").is_dir() - assert not (zf.filetree / "arr_2" / "0").exists() - assert not (zf.filetree / "arr_2" / "0").is_file() - assert not (zf.filetree / "arr_2" / "0").is_dir() - - -# V2 (``.zgroup`` / ``.zarray``) and V3 (``zarr.json``, ``c/`` layout, -# different default compressor) Zarr serialisations have different on-disk -# byte layouts and therefore different digests. Key expected values on the -# format that was *actually* produced rather than on ``zarr.__version__``: -# zarr-python 3.x can still write V2 via ``zarr_format=2``. -_ZARR_PROPERTIES_EXPECTED = { - "2": { - "total_size": 1516, - "total_digest": "4313ab36412db2981c3ed391b38604d6-5--1516", - "entries": [ - (".zgroup", 24, "e20297935e73dd0154104d4ea53040ab"), - ("arr_0", 746, "51c74ec257069ce3a555bdddeb50230a-2--746"), - ("arr_0/.zarray", 315, "9e30a0a1a465e24220d4132fdd544634"), - ("arr_0/0", 431, "ed4e934a474f1d2096846c6248f18c00"), - ("arr_1", 746, "7b99a0ad9bd8bb3331657e54755b1a31-2--746"), - ("arr_1/.zarray", 315, "9e30a0a1a465e24220d4132fdd544634"), - ("arr_1/0", 431, "fba4dee03a51bde314e9713b00284a93"), - ], - }, - "3": { - "total_size": 3935, - "total_digest": "00157f091c9a6295e89eb3c4c2efaeff-5--3935", - "entries": [ - ("arr_0", 2192, "ae16256ae750e4303674ccf1e23fa3c6-2--2192"), - ("arr_0/c", 1573, "93912a45f2107a08090f7b283297d662-1--1573"), - ("arr_0/c/0", 1573, "6c237f8d2d4a41bc1e26e31518dafd9e"), - ("arr_0/zarr.json", 619, "850fae056c97aa9c76df0a52411f4086"), - ("arr_1", 1677, "debc9ca4b2184a6ef1a3d6fcf7d79fd9-2--1677"), - ("arr_1/c", 1058, "2642f5d2df2cddf469313abd9910b371-1--1058"), - ("arr_1/c/0", 1058, "084d662af7251a807649fb48edc36e95"), - ("arr_1/zarr.json", 619, "850fae056c97aa9c76df0a52411f4086"), - ("zarr.json", 66, "457126c0639af2eba0140851c39c1aad"), - ], - }, -} - - -def test_zarr_properties(tmp_path: Path) -> None: - # Expected sizes and digests are selected by the Zarr serialisation - # format ``zarr.save`` actually produced (V2 vs V3 layouts differ). - filepath = tmp_path / "example.zarr" - dt = np.dtype(" None: - filepath = tmp_path / "example.zarr" - zarr.save(filepath, np.arange(1000), np.arange(1000, 0, -1)) - layout = TWO_ARRAY_ZARR_LAYOUT[zarr_format_of(filepath)] - subprocess.run(["git", "init"], cwd=str(filepath), check=True) - (filepath / ".dandi").mkdir() - (filepath / ".dandi" / "somefile.txt").write_text("Hello world!\n") - (filepath / ".gitattributes").write_text("* eol=lf\n") - (filepath / "arr_0" / ".gitmodules").write_text("# Empty\n") - (filepath / "arr_1" / ".datalad").mkdir() - (filepath / "arr_1" / ".datalad" / "config").write_text("# Empty\n") - zf = dandi_file(filepath) - assert isinstance(zf, ZarrAsset) - asset = zf.upload(new_dandiset.dandiset, {}) - assert isinstance(asset, RemoteZarrAsset) - local_entries = sorted(zf.iterfiles(include_dirs=True), key=attrgetter("parts")) - assert [str(e) for e in local_entries] == layout["files_and_dirs"] - remote_entries = sorted(asset.iterfiles(), key=attrgetter("parts")) - assert [str(e) for e in remote_entries] == layout["files"] - - -def test_upload_zarr_entry_content_type(new_dandiset, tmp_path): - filepath = tmp_path / "example.zarr" - zarr.save(filepath, np.arange(1000), np.arange(1000, 0, -1)) - root_meta = TWO_ARRAY_ZARR_LAYOUT[zarr_format_of(filepath)]["root_meta"] - zf = dandi_file(filepath) - assert isinstance(zf, ZarrAsset) - asset = zf.upload(new_dandiset.dandiset, {"description": "A test Zarr"}) - assert isinstance(asset, RemoteZarrAsset) - e = asset.get_entry_by_path(root_meta) - r = new_dandiset.client.get(e.download_url, json_resp=False) - assert r.headers["Content-Type"] == "application/json" - - -def test_validate_deep_zarr(tmp_path: Path) -> None: - zarr_path = tmp_path / "foo.zarr" - zarr.save(zarr_path, np.arange(1000), np.arange(1000, 0, -1)) - mkpaths(zarr_path, "a/b/c/d/e/f/g.txt") - zf = dandi_file(zarr_path) - assert zf.get_validation_errors() == [] - mkpaths(zarr_path, "a/b/c/d/e/f/g/h.txt") - assert [e.id for e in zf.get_validation_errors()] == [ - "dandi_zarr.tree_depth_exceeded" - ] - - -def test_validate_zarr_deep_via_excluded_dotfiles(tmp_path: Path) -> None: - zarr_path = tmp_path / "foo.zarr" - zarr.save(zarr_path, np.arange(1000), np.arange(1000, 0, -1)) - mkpaths(zarr_path, ".git/a/b/c/d/e/f/g.txt", "a/b/c/.git/d/e/f/g.txt") - zf = dandi_file(zarr_path) - assert zf.get_validation_errors() == [] - - -VALID_STORES_PATH = "data/zarr3_stores/valid_stores" - - -@pytest.mark.parametrize( - "path", - [ - "arrays_in_groups.zarr", - "single_array.zarr", - ], -) -def test_validate_valid_zarr3(path: str) -> None: - """ - Test validating valid Zarr format 3 objects, Zarr groups or arrays - - Parameters - ---------- - path : Path - The path to the store of the Zarr object in the filesystem relative to - `VALID_STORES_PATH` which is relative to the parent of the path of this - test file - """ - zf = dandi_file(Path(__file__).parent / VALID_STORES_PATH / path) - assert zf.get_validation_errors() == [] - - -INVALID_STORES_PATH = "data/zarr3_stores/invalid_stores" - - -@pytest.mark.parametrize( - "path, expected_result_ids", - [ - # Expects "zarr.cannot_open" because Zarr format version can't be determined - # without a zarr.json file - ("arrays_in_groups_missing_zarr_json.zarr", {"zarr.cannot_open"}), - ("single_array_missing_zarr_json.zarr", {"zarr.cannot_open"}), - # Stores with the `node_type` field in some zarr.json missing or having - # invalid values - ( - "arrays_in_groups_node_type_problem.zarr", - {"zarr.invalid_zarr_json", "zarr.invalid_zarr_json"}, - ), - ( - "single_array_node_type_problem.zarr", - {"zarr.invalid_zarr_json"}, - ), - # A store with a corrupt zarr.json for an array (missing fields other than - # `node_type`) - ("array_v3_corrupt_zarr_json.zarr", {"zarr.tensorstore_cannot_open"}), - ], -) -def test_validate_invalid_zarr3(path: str, expected_result_ids: set[str]) -> None: - """ - Test validating valid Zarr format 3 objects, Zarr groups or arrays - - Parameters - ---------- - path : Path - The path to the store of the Zarr object in the filesystem relative to - `INVALID_STORES_PATH` which is relative to the parent of the path of this - test file - """ - zf = dandi_file(Path(__file__).parent / INVALID_STORES_PATH / path) - - result_ids = {r.id for r in zf.get_validation_errors()} - assert result_ids == expected_result_ids +from __future__ import annotations + +from operator import attrgetter +import os +from pathlib import Path +import subprocess +from unittest.mock import ANY + +from dandischema.models import get_schema_version +import numpy as np +import pytest +import zarr + +from .fixtures import SampleDandiset +from .test_helpers import TWO_ARRAY_ZARR_LAYOUT, zarr_format_of +from .. import get_logger +from ..consts import ZARR_MIME_TYPE, dandiset_metadata_file +from ..dandiapi import AssetType, RemoteZarrAsset +from ..exceptions import UnknownAssetError +from ..files import ( + BIDSDatasetDescriptionAsset, + DandisetMetadataFile, + GenericAsset, + GenericBIDSAsset, + ImageAsset, + NWBAsset, + NWBBIDSAsset, + VideoAsset, + ZarrAsset, + ZarrBIDSAsset, + dandi_file, + find_dandi_files, + find_unused_paths, +) + +lgr = get_logger() + + +def mkpaths(root: Path, *paths: str) -> None: + for p in paths: + pp = root / p + pp.parent.mkdir(parents=True, exist_ok=True) + if p.endswith("/"): + pp.mkdir() + else: + pp.touch() + + +def test_find_dandi_files(tmp_path: Path) -> None: + mkpaths( + tmp_path, + dandiset_metadata_file, + "sample01.zarr/inner.nwb", + "sample01.zarr/foo", + "sample02.nwb", + "foo", + "bar.txt", + "subdir/sample03.nwb", + "subdir/sample04.zarr/inner2.nwb", + "subdir/sample04.zarr/baz", + "subdir/gnusto", + "subdir/cleesh.txt", + "empty.zarr/", + "glarch.mp4", + "quux.png", + ".ignored", + ".ignored.dir/ignored.nwb", + ) + + files = sorted( + find_dandi_files(tmp_path, dandiset_path=tmp_path), key=attrgetter("filepath") + ) + assert files == [ + VideoAsset( + filepath=tmp_path / "glarch.mp4", path="glarch.mp4", dandiset_path=tmp_path + ), + ImageAsset( + filepath=tmp_path / "quux.png", path="quux.png", dandiset_path=tmp_path + ), + ZarrAsset( + filepath=tmp_path / "sample01.zarr", + path="sample01.zarr", + dandiset_path=tmp_path, + ), + NWBAsset( + filepath=tmp_path / "sample02.nwb", + path="sample02.nwb", + dandiset_path=tmp_path, + ), + NWBAsset( + filepath=tmp_path / "subdir" / "sample03.nwb", + path="subdir/sample03.nwb", + dandiset_path=tmp_path, + ), + ZarrAsset( + filepath=tmp_path / "subdir" / "sample04.zarr", + path="subdir/sample04.zarr", + dandiset_path=tmp_path, + ), + ] + + files = sorted( + find_dandi_files(tmp_path, dandiset_path=tmp_path, allow_all=True), + key=attrgetter("filepath"), + ) + assert files == [ + GenericAsset( + filepath=tmp_path / "bar.txt", path="bar.txt", dandiset_path=tmp_path + ), + DandisetMetadataFile( + filepath=tmp_path / dandiset_metadata_file, dandiset_path=tmp_path + ), + GenericAsset(filepath=tmp_path / "foo", path="foo", dandiset_path=tmp_path), + VideoAsset( + filepath=tmp_path / "glarch.mp4", path="glarch.mp4", dandiset_path=tmp_path + ), + ImageAsset( + filepath=tmp_path / "quux.png", path="quux.png", dandiset_path=tmp_path + ), + ZarrAsset( + filepath=tmp_path / "sample01.zarr", + path="sample01.zarr", + dandiset_path=tmp_path, + ), + NWBAsset( + filepath=tmp_path / "sample02.nwb", + path="sample02.nwb", + dandiset_path=tmp_path, + ), + GenericAsset( + filepath=tmp_path / "subdir" / "cleesh.txt", + path="subdir/cleesh.txt", + dandiset_path=tmp_path, + ), + GenericAsset( + filepath=tmp_path / "subdir" / "gnusto", + path="subdir/gnusto", + dandiset_path=tmp_path, + ), + NWBAsset( + filepath=tmp_path / "subdir" / "sample03.nwb", + path="subdir/sample03.nwb", + dandiset_path=tmp_path, + ), + ZarrAsset( + filepath=tmp_path / "subdir" / "sample04.zarr", + path="subdir/sample04.zarr", + dandiset_path=tmp_path, + ), + ] + + files = sorted( + find_dandi_files(tmp_path, dandiset_path=tmp_path, include_metadata=True), + key=attrgetter("filepath"), + ) + assert files == [ + DandisetMetadataFile( + filepath=tmp_path / dandiset_metadata_file, dandiset_path=tmp_path + ), + VideoAsset( + filepath=tmp_path / "glarch.mp4", path="glarch.mp4", dandiset_path=tmp_path + ), + ImageAsset( + filepath=tmp_path / "quux.png", path="quux.png", dandiset_path=tmp_path + ), + ZarrAsset( + filepath=tmp_path / "sample01.zarr", + path="sample01.zarr", + dandiset_path=tmp_path, + ), + NWBAsset( + filepath=tmp_path / "sample02.nwb", + path="sample02.nwb", + dandiset_path=tmp_path, + ), + NWBAsset( + filepath=tmp_path / "subdir" / "sample03.nwb", + path="subdir/sample03.nwb", + dandiset_path=tmp_path, + ), + ZarrAsset( + filepath=tmp_path / "subdir" / "sample04.zarr", + path="subdir/sample04.zarr", + dandiset_path=tmp_path, + ), + ] + + +def test_find_unused_paths(tmp_path: Path) -> None: + (tmp_path / dandiset_metadata_file).touch() + (tmp_path / "known.nwb").touch() + (tmp_path / "unknown.txt").touch() + (tmp_path / "unknown-dir").mkdir() + (tmp_path / "unknown-dir" / "file.txt").touch() + (tmp_path / "mixed").mkdir() + (tmp_path / "mixed" / "known.nwb").touch() + (tmp_path / "mixed" / "sidecar.json").touch() + (tmp_path / "sample.zarr").mkdir() + (tmp_path / "sample.zarr" / "chunk").touch() + (tmp_path / "empty").mkdir() + (tmp_path / ".hidden").mkdir() + (tmp_path / ".hidden" / "secret.nwb").touch() + (tmp_path / "__MACOSX").mkdir() + (tmp_path / "__MACOSX" / "._known.nwb").touch() + (tmp_path / "Thumbs.db").touch() + + unused = find_unused_paths( + [tmp_path], + [ + tmp_path / dandiset_metadata_file, + tmp_path / "known.nwb", + tmp_path / "mixed" / "known.nwb", + tmp_path / "sample.zarr", + ], + dandiset_path=tmp_path, + ) + + assert [path.relative_to(tmp_path).as_posix() for path in unused] == [ + "mixed/sidecar.json", + "unknown-dir", + "unknown.txt", + ] + assert find_unused_paths( + [tmp_path / "mixed"], [tmp_path / "mixed" / "known.nwb"], dandiset_path=tmp_path + ) == [tmp_path / "mixed" / "sidecar.json"] + + +def test_find_unused_paths_ignores_symlinked_directory(tmp_path: Path) -> None: + target = tmp_path / "outside" + target.mkdir() + (target / "omitted.txt").touch() + symlink = tmp_path / "linked" + try: + symlink.symlink_to(target, target_is_directory=True) + except OSError as exc: + pytest.skip(f"cannot create directory symlink: {exc}") + + assert find_unused_paths([symlink], [], dandiset_path=tmp_path) == [] + + +@pytest.mark.ai_generated +def test_find_unused_paths_handles_missing_and_ignored_entries(tmp_path: Path) -> None: + """Only existing, user-visible paths should be reported as omitted.""" + (tmp_path / dandiset_metadata_file).touch() + visible = tmp_path / "notes.txt" + visible.write_text("notes") + missing = tmp_path / "not-created.txt" + hidden = tmp_path / ".hidden.txt" + hidden.touch() + + assert find_unused_paths( + [visible, missing, hidden, tmp_path / dandiset_metadata_file], + [], + dandiset_path=tmp_path, + ) == [visible] + + +@pytest.mark.ai_generated +def test_find_unused_paths_rejects_paths_outside_dandiset(tmp_path: Path) -> None: + outside = tmp_path.parent / "outside.txt" + outside.touch() + + with pytest.raises(ValueError, match="not inside Dandiset path"): + find_unused_paths([outside], [], dandiset_path=tmp_path) + + +def test_find_dandi_files_with_bids(tmp_path: Path) -> None: + mkpaths( + tmp_path, + dandiset_metadata_file, + "foo.txt", + "bar.nwb", + "bids1/.bidsignore", + "bids1/dataset_description.json", + "bids1/file.txt", + "bids1/subdir/quux.nwb", + "bids1/subdir/glarch.zarr/dataset_description.json", + "bids2/dataset_description.json", + "bids2/movie.mp4", + "bids2/subbids/dataset_description.json", + "bids2/subbids/data.json", + ) + + files = sorted( + find_dandi_files(tmp_path, dandiset_path=tmp_path, allow_all=False), + key=attrgetter("filepath"), + ) + + assert files == [ + NWBAsset(filepath=tmp_path / "bar.nwb", path="bar.nwb", dandiset_path=tmp_path), + GenericBIDSAsset( + filepath=tmp_path / "bids1" / ".bidsignore", + path="bids1/.bidsignore", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + BIDSDatasetDescriptionAsset( + filepath=tmp_path / "bids1" / "dataset_description.json", + path="bids1/dataset_description.json", + dandiset_path=tmp_path, + dataset_files=ANY, # type: ignore[arg-type] + ), + GenericBIDSAsset( + filepath=tmp_path / "bids1" / "file.txt", + path="bids1/file.txt", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + ZarrBIDSAsset( + filepath=tmp_path / "bids1" / "subdir" / "glarch.zarr", + path="bids1/subdir/glarch.zarr", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + NWBBIDSAsset( + filepath=tmp_path / "bids1" / "subdir" / "quux.nwb", + path="bids1/subdir/quux.nwb", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + BIDSDatasetDescriptionAsset( + filepath=tmp_path / "bids2" / "dataset_description.json", + path="bids2/dataset_description.json", + dandiset_path=tmp_path, + dataset_files=ANY, # type: ignore[arg-type] + ), + GenericBIDSAsset( + filepath=tmp_path / "bids2" / "movie.mp4", + path="bids2/movie.mp4", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + GenericBIDSAsset( + filepath=tmp_path / "bids2" / "subbids" / "data.json", + path="bids2/subbids/data.json", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + GenericBIDSAsset( + filepath=tmp_path / "bids2" / "subbids" / "dataset_description.json", + path="bids2/subbids/dataset_description.json", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + ] + + bidsdd = files[2] + assert isinstance(bidsdd, BIDSDatasetDescriptionAsset) + assert sorted(bidsdd.dataset_files, key=attrgetter("filepath")) == [ + GenericBIDSAsset( + filepath=tmp_path / "bids1" / ".bidsignore", + path="bids1/.bidsignore", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + GenericBIDSAsset( + filepath=tmp_path / "bids1" / "file.txt", + path="bids1/file.txt", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + ZarrBIDSAsset( + filepath=tmp_path / "bids1" / "subdir" / "glarch.zarr", + path="bids1/subdir/glarch.zarr", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + NWBBIDSAsset( + filepath=tmp_path / "bids1" / "subdir" / "quux.nwb", + path="bids1/subdir/quux.nwb", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + ] + for asset in bidsdd.dataset_files: + assert asset.bids_dataset_description is bidsdd + + bidsdd = files[6] + assert isinstance(bidsdd, BIDSDatasetDescriptionAsset) + assert sorted(bidsdd.dataset_files, key=attrgetter("filepath")) == [ + GenericBIDSAsset( + filepath=tmp_path / "bids2" / "movie.mp4", + path="bids2/movie.mp4", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + GenericBIDSAsset( + filepath=tmp_path / "bids2" / "subbids" / "data.json", + path="bids2/subbids/data.json", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + GenericBIDSAsset( + filepath=tmp_path / "bids2" / "subbids" / "dataset_description.json", + path="bids2/subbids/dataset_description.json", + dandiset_path=tmp_path, + bids_dataset_description_ref=ANY, # type: ignore[arg-type] + ), + ] + for asset in bidsdd.dataset_files: + assert asset.bids_dataset_description is bidsdd + + +# This test sometimes fails and sometimes passes when running on NFS. +@pytest.mark.flaky(reruns=10) +def test_dandi_file_zarr_with_excluded_dotfiles(tmp_path: Path) -> None: + zarr_path = tmp_path / "foo.zarr" + mkpaths( + zarr_path, + ".git/data", + ".gitattributes", + ".dandi/somefile.txt", + ".datalad/", + "arr_0/.gitmodules", + ) + with pytest.raises(UnknownAssetError): + dandi_file(zarr_path) + with (zarr_path / "arr_0" / "foo").open("w") as fp: + print("Text.", file=fp) + # Force changes to be synced when testing on NFS: + fp.flush() + os.fsync(fp.fileno()) + zf = dandi_file(zarr_path) + assert isinstance(zf, ZarrAsset) + + +def test_validate_simple1(simple1_nwb: Path) -> None: + # this file should be ok as long as schema_version is specified + errors = dandi_file(simple1_nwb).get_validation_errors( + schema_version=get_schema_version() + ) + assert errors == [] + + +def test_validate_simple1_no_subject(simple1_nwb: Path) -> None: + errors = dandi_file(simple1_nwb).get_validation_errors() + errmsgs = [] + for e in errors: + assert e.message is not None + errmsgs.append(e.message) + assert errmsgs == ["Subject is missing."] + + +def test_validate_simple2(organized_nwb_dir: Path) -> None: + # this file should be ok since a Subject is included + errors = dandi_file( + organized_nwb_dir / "sub-mouse001" / "sub-mouse001.nwb", + dandiset_path=organized_nwb_dir, + ).get_validation_errors() + assert not errors + + +def test_validate_simple2_new(organized_nwb_dir: Path) -> None: + # this file should be ok + errors = dandi_file( + organized_nwb_dir / "sub-mouse001" / "sub-mouse001.nwb", + dandiset_path=organized_nwb_dir, + ).get_validation_errors(schema_version=get_schema_version()) + assert not errors + + +def test_validate_simple3_no_subject_id(simple3_nwb: Path) -> None: + errors = dandi_file(simple3_nwb).get_validation_errors() + errmsgs = [] + for e in errors: + assert e.message is not None + errmsgs.append(e.message) + assert errmsgs == ["subject_id is missing."] + + +def test_validate_bogus(tmp_path): + """ + Notes + ----- + * Intended to produce use-case for https://github.com/dandi/dandi-cli/issues/93 + but it would be tricky, so it is more of a smoke test that + we do not crash + """ + path = tmp_path / "wannabe.nwb" + path.write_text("not really nwb") + errors = dandi_file(path).get_validation_errors() + # ATM we would get 2 errors -- since could not be open in two places, + # but that would be too rigid to test. Let's just see that we have expected errors + assert any( + e.message.startswith( + ( + "Unable to open file", + "Unable to synchronously open file", + "Could not find an IO to read the file", + ) + ) + for e in errors + ) + # Recent versions of hdf5 changed the error message, hence the need to + # check for two different patterns. + + +def test_upload_zarr(new_dandiset, tmp_path): + filepath = tmp_path / "example.zarr" + zarr.save(filepath, np.arange(1000), np.arange(1000, 0, -1)) + layout = TWO_ARRAY_ZARR_LAYOUT[zarr_format_of(filepath)] + root_meta = layout["root_meta"] + zf = dandi_file(filepath) + assert isinstance(zf, ZarrAsset) + asset = zf.upload(new_dandiset.dandiset, {"description": "A test Zarr"}) + assert isinstance(asset, RemoteZarrAsset) + assert asset.asset_type is AssetType.ZARR + assert asset.path == "example.zarr" + md = asset.get_raw_metadata() + assert md["encodingFormat"] == ZARR_MIME_TYPE + assert md["description"] == "A test Zarr" + md["description"] = "A modified Zarr" + asset.set_raw_metadata(md) + md = asset.get_raw_metadata() + assert md["description"] == "A modified Zarr" + + entries = sorted(asset.iterfiles(), key=attrgetter("parts")) + assert [str(e) for e in entries] == layout["files"] + + entries = sorted(zf.iterfiles(include_dirs=True), key=attrgetter("parts")) + assert [str(e) for e in entries] == layout["files_and_dirs"] + # The root group metadata file is ``.zgroup`` in V2 and ``zarr.json`` in + # V3; either way it must be a real file at the Zarr root. + assert (zf.filetree / root_meta).exists() + assert (zf.filetree / root_meta).is_file() + assert not (zf.filetree / root_meta).is_dir() + assert (zf.filetree / "arr_0").exists() + assert not (zf.filetree / "arr_0").is_file() + assert (zf.filetree / "arr_0").is_dir() + assert not (zf.filetree / "0").exists() + assert not (zf.filetree / "0").is_file() + assert not (zf.filetree / "0").is_dir() + # ``arr_0/.zgroup`` never exists: in V2 ``arr_0`` is an array (uses + # ``.zarray``); in V3 ``.zgroup`` is not used at all. + assert not (zf.filetree / "arr_0" / ".zgroup").exists() + assert not (zf.filetree / "arr_0" / ".zgroup").is_file() + assert not (zf.filetree / "arr_0" / ".zgroup").is_dir() + assert not (zf.filetree / ".zgroup" / "0").exists() + assert not (zf.filetree / ".zgroup" / "0").is_file() + assert not (zf.filetree / ".zgroup" / "0").is_dir() + assert not (zf.filetree / "arr_2" / "0").exists() + assert not (zf.filetree / "arr_2" / "0").is_file() + assert not (zf.filetree / "arr_2" / "0").is_dir() + + +# V2 (``.zgroup`` / ``.zarray``) and V3 (``zarr.json``, ``c/`` layout, +# different default compressor) Zarr serialisations have different on-disk +# byte layouts and therefore different digests. Key expected values on the +# format that was *actually* produced rather than on ``zarr.__version__``: +# zarr-python 3.x can still write V2 via ``zarr_format=2``. +_ZARR_PROPERTIES_EXPECTED = { + "2": { + "total_size": 1516, + "total_digest": "4313ab36412db2981c3ed391b38604d6-5--1516", + "entries": [ + (".zgroup", 24, "e20297935e73dd0154104d4ea53040ab"), + ("arr_0", 746, "51c74ec257069ce3a555bdddeb50230a-2--746"), + ("arr_0/.zarray", 315, "9e30a0a1a465e24220d4132fdd544634"), + ("arr_0/0", 431, "ed4e934a474f1d2096846c6248f18c00"), + ("arr_1", 746, "7b99a0ad9bd8bb3331657e54755b1a31-2--746"), + ("arr_1/.zarray", 315, "9e30a0a1a465e24220d4132fdd544634"), + ("arr_1/0", 431, "fba4dee03a51bde314e9713b00284a93"), + ], + }, + "3": { + "total_size": 3935, + "total_digest": "00157f091c9a6295e89eb3c4c2efaeff-5--3935", + "entries": [ + ("arr_0", 2192, "ae16256ae750e4303674ccf1e23fa3c6-2--2192"), + ("arr_0/c", 1573, "93912a45f2107a08090f7b283297d662-1--1573"), + ("arr_0/c/0", 1573, "6c237f8d2d4a41bc1e26e31518dafd9e"), + ("arr_0/zarr.json", 619, "850fae056c97aa9c76df0a52411f4086"), + ("arr_1", 1677, "debc9ca4b2184a6ef1a3d6fcf7d79fd9-2--1677"), + ("arr_1/c", 1058, "2642f5d2df2cddf469313abd9910b371-1--1058"), + ("arr_1/c/0", 1058, "084d662af7251a807649fb48edc36e95"), + ("arr_1/zarr.json", 619, "850fae056c97aa9c76df0a52411f4086"), + ("zarr.json", 66, "457126c0639af2eba0140851c39c1aad"), + ], + }, +} + + +def test_zarr_properties(tmp_path: Path) -> None: + # Expected sizes and digests are selected by the Zarr serialisation + # format ``zarr.save`` actually produced (V2 vs V3 layouts differ). + filepath = tmp_path / "example.zarr" + dt = np.dtype(" None: + filepath = tmp_path / "example.zarr" + zarr.save(filepath, np.arange(1000), np.arange(1000, 0, -1)) + layout = TWO_ARRAY_ZARR_LAYOUT[zarr_format_of(filepath)] + subprocess.run(["git", "init"], cwd=str(filepath), check=True) + (filepath / ".dandi").mkdir() + (filepath / ".dandi" / "somefile.txt").write_text("Hello world!\n") + (filepath / ".gitattributes").write_text("* eol=lf\n") + (filepath / "arr_0" / ".gitmodules").write_text("# Empty\n") + (filepath / "arr_1" / ".datalad").mkdir() + (filepath / "arr_1" / ".datalad" / "config").write_text("# Empty\n") + zf = dandi_file(filepath) + assert isinstance(zf, ZarrAsset) + asset = zf.upload(new_dandiset.dandiset, {}) + assert isinstance(asset, RemoteZarrAsset) + local_entries = sorted(zf.iterfiles(include_dirs=True), key=attrgetter("parts")) + assert [str(e) for e in local_entries] == layout["files_and_dirs"] + remote_entries = sorted(asset.iterfiles(), key=attrgetter("parts")) + assert [str(e) for e in remote_entries] == layout["files"] + + +def test_upload_zarr_entry_content_type(new_dandiset, tmp_path): + filepath = tmp_path / "example.zarr" + zarr.save(filepath, np.arange(1000), np.arange(1000, 0, -1)) + root_meta = TWO_ARRAY_ZARR_LAYOUT[zarr_format_of(filepath)]["root_meta"] + zf = dandi_file(filepath) + assert isinstance(zf, ZarrAsset) + asset = zf.upload(new_dandiset.dandiset, {"description": "A test Zarr"}) + assert isinstance(asset, RemoteZarrAsset) + e = asset.get_entry_by_path(root_meta) + r = new_dandiset.client.get(e.download_url, json_resp=False) + assert r.headers["Content-Type"] == "application/json" + + +def test_validate_deep_zarr(tmp_path: Path) -> None: + zarr_path = tmp_path / "foo.zarr" + zarr.save(zarr_path, np.arange(1000), np.arange(1000, 0, -1)) + mkpaths(zarr_path, "a/b/c/d/e/f/g.txt") + zf = dandi_file(zarr_path) + assert zf.get_validation_errors() == [] + mkpaths(zarr_path, "a/b/c/d/e/f/g/h.txt") + assert [e.id for e in zf.get_validation_errors()] == [ + "dandi_zarr.tree_depth_exceeded" + ] + + +def test_validate_zarr_deep_via_excluded_dotfiles(tmp_path: Path) -> None: + zarr_path = tmp_path / "foo.zarr" + zarr.save(zarr_path, np.arange(1000), np.arange(1000, 0, -1)) + mkpaths(zarr_path, ".git/a/b/c/d/e/f/g.txt", "a/b/c/.git/d/e/f/g.txt") + zf = dandi_file(zarr_path) + assert zf.get_validation_errors() == [] + + +VALID_STORES_PATH = "data/zarr3_stores/valid_stores" + + +@pytest.mark.parametrize( + "path", + [ + "arrays_in_groups.zarr", + "single_array.zarr", + ], +) +def test_validate_valid_zarr3(path: str) -> None: + """ + Test validating valid Zarr format 3 objects, Zarr groups or arrays + + Parameters + ---------- + path : Path + The path to the store of the Zarr object in the filesystem relative to + `VALID_STORES_PATH` which is relative to the parent of the path of this + test file + """ + zf = dandi_file(Path(__file__).parent / VALID_STORES_PATH / path) + assert zf.get_validation_errors() == [] + + +INVALID_STORES_PATH = "data/zarr3_stores/invalid_stores" + + +@pytest.mark.parametrize( + "path, expected_result_ids", + [ + # Expects "zarr.cannot_open" because Zarr format version can't be determined + # without a zarr.json file + ("arrays_in_groups_missing_zarr_json.zarr", {"zarr.cannot_open"}), + ("single_array_missing_zarr_json.zarr", {"zarr.cannot_open"}), + # Stores with the `node_type` field in some zarr.json missing or having + # invalid values + ( + "arrays_in_groups_node_type_problem.zarr", + {"zarr.invalid_zarr_json", "zarr.invalid_zarr_json"}, + ), + ( + "single_array_node_type_problem.zarr", + {"zarr.invalid_zarr_json"}, + ), + # A store with a corrupt zarr.json for an array (missing fields other than + # `node_type`) + ("array_v3_corrupt_zarr_json.zarr", {"zarr.tensorstore_cannot_open"}), + ], +) +def test_validate_invalid_zarr3(path: str, expected_result_ids: set[str]) -> None: + """ + Test validating valid Zarr format 3 objects, Zarr groups or arrays + + Parameters + ---------- + path : Path + The path to the store of the Zarr object in the filesystem relative to + `INVALID_STORES_PATH` which is relative to the parent of the path of this + test file + """ + zf = dandi_file(Path(__file__).parent / INVALID_STORES_PATH / path) + + result_ids = {r.id for r in zf.get_validation_errors()} + assert result_ids == expected_result_ids From bbec8c3cd97a51521dc96bf8d64b12eac6afb9c2 Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Wed, 9 Sep 2026 21:55:30 +0330 Subject: [PATCH 5/5] test: mark upload omission coverage as generated --- dandi/tests/test_upload.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index 11cc2aa3c..d2d799015 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -314,6 +314,7 @@ def test_upload_bids_non_nwb_file(bids_dandiset: SampleDandiset) -> None: assert [asset.path for asset in bids_dandiset.dandiset.get_assets()] == ["README"] +@pytest.mark.ai_generated def test_upload_warns_for_unrecognized_paths( caplog: pytest.LogCaptureFixture, new_dandiset: SampleDandiset, @@ -335,6 +336,7 @@ def test_upload_warns_for_unrecognized_paths( ) in caplog.text +@pytest.mark.ai_generated def test_upload_partial_does_not_warn_for_unrequested_paths( caplog: pytest.LogCaptureFixture, new_dandiset: SampleDandiset, @@ -352,6 +354,7 @@ def test_upload_partial_does_not_warn_for_unrequested_paths( assert "were not uploaded because they were not recognized" not in caplog.text +@pytest.mark.ai_generated def test_upload_allow_any_path_suppresses_omission_warning( caplog: pytest.LogCaptureFixture, new_dandiset: SampleDandiset ) -> None: @@ -363,6 +366,7 @@ def test_upload_allow_any_path_suppresses_omission_warning( assert "were not uploaded because they were not recognized" not in caplog.text +@pytest.mark.ai_generated def test_upload_omission_warning_survives_upload_error( caplog: pytest.LogCaptureFixture, mocker: MockerFixture,