From a13100f0ccd1905b58516ae9c04967eeb96b2276 Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Fri, 4 Sep 2026 02:19:17 +0330 Subject: [PATCH 1/9] 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 ee7714042a79bbd1cbccf8560754826fb1ad6b1e Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Sat, 5 Sep 2026 22:17:44 +0330 Subject: [PATCH 2/9] 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 e9a7a1ee195d8aa5c1dccbe7000fd96d52eb74cb Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Sat, 5 Sep 2026 22:40:43 +0330 Subject: [PATCH 3/9] 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 e19642cc33faf915ded9f183cde541795bf07ef0 Mon Sep 17 00:00:00 2001 From: Amirali Moradniaei Date: Wed, 9 Sep 2026 21:14:25 +0330 Subject: [PATCH 4/9] 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 023dfcb7b4f365fa7f7cf705b4a3562553b57351 Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Wed, 9 Sep 2026 21:55:30 +0330 Subject: [PATCH 5/9] 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, From de00f6ce877dd30d97863c1861fa214f61d531fe Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Sat, 12 Sep 2026 01:02:46 +0330 Subject: [PATCH 6/9] fix: use matching pronouns in upload omission warning --- dandi/upload.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dandi/upload.py b/dandi/upload.py index f57e7edbf..ef90a85eb 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -480,11 +480,13 @@ def report_omitted_paths() -> None: path.relative_to(dandiset.path).as_posix() for path in omitted_paths ] verb = "was" if len(relpaths) == 1 else "were" + pronoun = "it was" if len(relpaths) == 1 else "they were" lgr.warning( - "%s %s not uploaded because they were not recognized as DANDI " + "%s %s not uploaded because %s not recognized as DANDI " "assets: %s. Review the paths or use --allow-any-path if intentional.", pluralize(len(relpaths), "path"), verb, + pronoun, ", ".join(relpaths[:10]) + (", ..." if len(relpaths) > 10 else ""), ) lgr.debug("Complete list of paths not uploaded: %s", ", ".join(relpaths)) From a43637229accb75022940cd9c75a901421766619 Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Sat, 12 Sep 2026 01:23:43 +0330 Subject: [PATCH 7/9] test: tighten omission assertions and normalize line endings --- dandi/tests/test_files.py | 1454 ++++++++++++++++++------------------ dandi/tests/test_upload.py | 4 +- 2 files changed, 729 insertions(+), 729 deletions(-) diff --git a/dandi/tests/test_files.py b/dandi/tests/test_files.py index 28ce7841a..cdbcbedff 100644 --- a/dandi/tests/test_files.py +++ b/dandi/tests/test_files.py @@ -1,727 +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) == [] - - -@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 __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 diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index d2d799015..8f8ca75cb 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -351,7 +351,7 @@ def test_upload_partial_does_not_warn_for_unrequested_paths( 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 + assert "not recognized as DANDI assets" not in caplog.text @pytest.mark.ai_generated @@ -363,7 +363,7 @@ def test_upload_allow_any_path_suppresses_omission_warning( 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 + assert "not recognized as DANDI assets" not in caplog.text @pytest.mark.ai_generated From 9f409099706cc6335cb2d5a8362ab18d9d607cd8 Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Tue, 15 Sep 2026 02:24:42 +0330 Subject: [PATCH 8/9] Reuse asset discovery for upload omission warnings --- dandi/files/__init__.py | 88 +--------------------- dandi/tests/test_files.py | 79 -------------------- dandi/tests/test_upload.py | 26 ++++--- dandi/tests/test_upload_discovery.py | 108 +++++++++++++++++++++++++++ dandi/upload.py | 72 +++++++++++------- 5 files changed, 170 insertions(+), 203 deletions(-) create mode 100644 dandi/tests/test_upload_discovery.py diff --git a/dandi/files/__init__.py b/dandi/files/__init__.py index 439614db9..8b1d501bc 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 Iterable, Iterator +from collections.abc import Iterator import os.path from pathlib import Path @@ -66,13 +66,10 @@ "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, @@ -164,89 +161,6 @@ 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 cdbcbedff..bc73aece2 100644 --- a/dandi/tests/test_files.py +++ b/dandi/tests/test_files.py @@ -30,7 +30,6 @@ ZarrBIDSAsset, dandi_file, find_dandi_files, - find_unused_paths, ) lgr = get_logger() @@ -186,84 +185,6 @@ 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) == [] - - -@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, diff --git a/dandi/tests/test_upload.py b/dandi/tests/test_upload.py index 8f8ca75cb..12e533202 100644 --- a/dandi/tests/test_upload.py +++ b/dandi/tests/test_upload.py @@ -330,10 +330,11 @@ def test_upload_warns_for_unrecognized_paths( 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 + (record,) = [ + r for r in caplog.records if "not recognized as DANDI assets" in r.message + ] + assert isinstance(record.args, tuple) + assert record.args[-1] == "notes, sidecar.json" @pytest.mark.ai_generated @@ -356,12 +357,10 @@ def test_upload_partial_does_not_warn_for_unrequested_paths( @pytest.mark.ai_generated def test_upload_allow_any_path_suppresses_omission_warning( - caplog: pytest.LogCaptureFixture, new_dandiset: SampleDandiset + caplog: pytest.LogCaptureFixture, text_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) + text_dandiset.upload() assert "not recognized as DANDI assets" not in caplog.text @@ -381,12 +380,17 @@ def test_upload_omission_warning_survives_upload_error( LocalFileAsset, "iter_upload", side_effect=UploadError("upload failed") ) - with caplog.at_level("WARNING", logger="dandi"), pytest.raises( - UploadError, match="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 + (record,) = [ + r for r in caplog.records if "not recognized as DANDI assets" in r.message + ] + assert isinstance(record.args, tuple) + assert record.args[-1] == "sidecar.json" @sweep_embargo diff --git a/dandi/tests/test_upload_discovery.py b/dandi/tests/test_upload_discovery.py new file mode 100644 index 000000000..46d6678ad --- /dev/null +++ b/dandi/tests/test_upload_discovery.py @@ -0,0 +1,108 @@ +from pathlib import Path, PurePosixPath + +import pytest +from pytest_mock import MockerFixture + +from .test_files import mkpaths +from ..dandiset import Dandiset +from ..files import find_dandi_files +from ..upload import _partition_upload_assets, upload + + +@pytest.mark.ai_generated +@pytest.mark.parametrize("allow_any_path", [False, True]) +@pytest.mark.parametrize( + "roots, expected", + [ + (["."], ["Thumbs.db", "mixed/sidecar.json", "notes", "unknown.txt"]), + (["mixed"], ["mixed/sidecar.json"]), + (["notes"], ["notes/nested"]), + (["notes/nested/readme.txt"], ["notes/nested/readme.txt"]), + (["mixed", "mixed/known.nwb"], ["mixed/sidecar.json"]), + (["sample.zarr"], []), + (["empty", ".hidden"], []), + ], +) +def test_partition_matches_discovery( + tmp_path: Path, roots: list[str], expected: list[str], allow_any_path: bool +) -> None: + mkpaths( + tmp_path, + "dandiset.yaml", + "known.nwb", + "unknown.txt", + "Thumbs.db", + "notes/nested/readme.txt", + "mixed/known.nwb", + "mixed/sidecar.json", + "sample.zarr/chunk", + "empty/", + ".hidden/secret.nwb", + "__MACOSX/._known.nwb", + ) + ds = Dandiset(tmp_path) + assets = ds.assets(allow_all=True) + selected, omitted = _partition_upload_assets( + assets, [PurePosixPath(p) for p in roots], allow_any_path + ) + # Compare against the real discovery pipeline, including request scoping. + baseline = ds.assets(allow_all=allow_any_path) + assert sorted(a.path for a in selected) == sorted( + {a.path for root in roots for a in baseline.under_paths([root])} + ) + assert [str(p) for p in omitted] == ([] if allow_any_path else expected) + + +@pytest.mark.ai_generated +def test_wholly_unrecognized_dandiset(tmp_path: Path) -> None: + mkpaths(tmp_path, "dandiset.yaml", "a.txt", "notes/readme.txt") + selected, omitted = _partition_upload_assets( + Dandiset(tmp_path).assets(allow_all=True), [PurePosixPath(".")], False + ) + assert selected == [] + assert omitted == [PurePosixPath("a.txt"), PurePosixPath("notes")] + + +@pytest.mark.ai_generated +def test_bids_assets_stay_recognized(tmp_path: Path) -> None: + mkpaths(tmp_path, "dandiset.yaml", "dataset_description.json", "sidecar.json") + assets = Dandiset(tmp_path).assets(allow_all=True) + selected, omitted = _partition_upload_assets(assets, [PurePosixPath(".")], False) + assert sorted(a.filepath for a in selected) == sorted( + a.filepath for a in find_dandi_files(tmp_path, dandiset_path=tmp_path) + ) + assert omitted == [] + + +@pytest.mark.ai_generated +def test_symlinked_directory_is_not_an_omitted_asset(tmp_path: Path) -> None: + mkpaths(tmp_path, "dandiset.yaml", "target/notes.txt") + try: + (tmp_path / "linked").symlink_to(tmp_path / "target", target_is_directory=True) + except OSError as exc: + pytest.skip(f"Cannot create directory symlink: {exc}") + selected, omitted = _partition_upload_assets( + Dandiset(tmp_path).assets(allow_all=True), [PurePosixPath("linked")], False + ) + assert selected == [] + assert omitted == [] + + +@pytest.mark.ai_generated +@pytest.mark.parametrize("count", [1, 12]) +def test_upload_unknown_only_reports_paths( + tmp_path: Path, mocker: MockerFixture, caplog: pytest.LogCaptureFixture, count: int +) -> None: + mkpaths(tmp_path, "dandiset.yaml", *(f"note-{i:02}.txt" for i in range(count))) + (tmp_path / "dandiset.yaml").write_text("identifier: '000001'\n") + mocker.patch("dandi.upload.DandiAPIClient.for_dandi_instance") + upload([tmp_path]) + (record,) = [ + r for r in caplog.records if "not recognized as DANDI assets" in r.message + ] + expected = ", ".join(f"note-{i:02}.txt" for i in range(min(count, 10))) + if count > 10: + expected += ", ..." + assert isinstance(record.args, tuple) + assert record.args[-1] == expected + assert "note-11.txt" in caplog.text if count > 10 else "note-00.txt" in caplog.text diff --git a/dandi/upload.py b/dandi/upload.py index ef90a85eb..911bfe732 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -17,7 +17,7 @@ from enum import StrEnum import io import os.path -from pathlib import Path +from pathlib import Path, PurePosixPath import re import time from time import sleep @@ -37,15 +37,15 @@ dandiset_metadata_file, ) from .dandiapi import DandiAPIClient, RemoteAsset -from .dandiset import Dandiset +from .dandiset import AssetView, Dandiset from .exceptions import NotFoundError, UploadError, UploadValidationError from .files import ( DandiFile, DandisetMetadataFile, + GenericAsset, LocalAsset, LocalDirectoryAsset, ZarrAsset, - find_unused_paths, ) from .misctypes import Digest from .support import pyout as pyouts @@ -55,6 +55,39 @@ from .validate._types import Severity +def _partition_upload_assets( + assets: AssetView, roots: Sequence[PurePosixPath], allow_any_path: bool +) -> tuple[list[LocalAsset], list[PurePosixPath]]: + """Select uploads and collapse omitted paths using the existing discovery.""" + root_set = set(roots) + roots = [p for p in root_set if not any(a in root_set for a in p.parents)] + selected = [] + omitted = [] + for asset in assets.under_paths(roots): + if type(asset) is GenericAsset and not allow_any_path: + omitted.append(PurePosixPath(asset.path)) + else: + selected.append(asset) + keep = { + ancestor + for asset in selected + for ancestor in (PurePosixPath(asset.path), *PurePosixPath(asset.path).parents) + } + boundaries = set(roots) | {PurePosixPath(".")} + collapsed = set() + for path in omitted: + candidate = path + if path in boundaries: + collapsed.add(path) + continue + for ancestor in path.parents: + if ancestor in boundaries or ancestor in keep: + break + candidate = ancestor + collapsed.add(candidate) + return selected, sorted(collapsed) + + def _check_dandidownload_paths(dfile: DandiFile) -> None: """ Check if an asset contains .dandidownload paths and raise UploadError if found. @@ -236,26 +269,19 @@ def new_super_len(o: Any) -> int: # DO NOT FACTOR OUT THIS VARIABLE! It stores any # BIDSDatasetDescriptionAsset instances for the Dandiset, which need to # remain alive until we're done working with all BIDS assets. - assets = dandiset.assets(allow_all=allow_any_path) + assets = dandiset.assets(allow_all=True) + selected, omitted_paths = _partition_upload_assets( + assets, + [PurePosixPath(Path(p).relative_to(dandiset.path)) for p in paths], + allow_any_path, + ) dandi_files: list[DandiFile] = [] # Build the list step by step so as not to confuse mypy dandi_files.append(dandiset.metadata_file()) - dandi_files.extend( - assets.under_paths(Path(p).relative_to(dandiset.path) for p in paths) - ) + dandi_files.extend(selected) 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 @@ -476,17 +502,11 @@ def report_omitted_paths() -> None: if not omitted_paths: return - relpaths = [ - path.relative_to(dandiset.path).as_posix() for path in omitted_paths - ] - verb = "was" if len(relpaths) == 1 else "were" - pronoun = "it was" if len(relpaths) == 1 else "they were" + relpaths = [path.as_posix() for path in omitted_paths] lgr.warning( - "%s %s not uploaded because %s not recognized as DANDI " - "assets: %s. Review the paths or use --allow-any-path if intentional.", + "%s not uploaded (not recognized as DANDI assets): %s. " + "Review the paths or use --allow-any-path if intentional.", pluralize(len(relpaths), "path"), - verb, - pronoun, ", ".join(relpaths[:10]) + (", ..." if len(relpaths) > 10 else ""), ) lgr.debug("Complete list of paths not uploaded: %s", ", ".join(relpaths)) From 585b8d6eb406f4a512949eaae48f93420bbf8fd4 Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Wed, 16 Sep 2026 12:49:41 +0330 Subject: [PATCH 9/9] TEST: cover nested upload roots --- dandi/tests/test_upload_discovery.py | 4 ++++ dandi/upload.py | 2 ++ 2 files changed, 6 insertions(+) diff --git a/dandi/tests/test_upload_discovery.py b/dandi/tests/test_upload_discovery.py index 46d6678ad..4db6a1bcd 100644 --- a/dandi/tests/test_upload_discovery.py +++ b/dandi/tests/test_upload_discovery.py @@ -19,6 +19,10 @@ (["notes"], ["notes/nested"]), (["notes/nested/readme.txt"], ["notes/nested/readme.txt"]), (["mixed", "mixed/known.nwb"], ["mixed/sidecar.json"]), + ( + [".", "mixed/known.nwb"], + ["Thumbs.db", "mixed/sidecar.json", "notes", "unknown.txt"], + ), (["sample.zarr"], []), (["empty", ".hidden"], []), ], diff --git a/dandi/upload.py b/dandi/upload.py index 911bfe732..1727f4b12 100644 --- a/dandi/upload.py +++ b/dandi/upload.py @@ -60,6 +60,8 @@ def _partition_upload_assets( ) -> tuple[list[LocalAsset], list[PurePosixPath]]: """Select uploads and collapse omitted paths using the existing discovery.""" root_set = set(roots) + # This pruning is required: under_paths() can otherwise let a nested root + # narrow the selection when it is supplied alongside its parent. roots = [p for p in root_set if not any(a in root_set for a in p.parents)] selected = [] omitted = []