Skip to content

Commit 5b292aa

Browse files
committed
Add --missing-file-content=stream to validate DataLad Dandisets by streaming annexed content
`dandi validate` can now validate a DataLad/git-annex Dandiset (such as the clones at https://github.com/dandisets) whose content has not been fetched: with `--missing-file-content=stream`, the content of each annexed file is streamed on demand (via fsspec) from the URLs registered for it in git-annex, preferring direct S3 URLs over DANDI API download URLs, so that pynwb and nwbinspector run without downloading the (possibly terabytes of) data. - New `dandi.support.annex` module: parses git-annex keys from the broken symlinks, reads URL logs from the `git-annex` branch (local or remote-tracking) using only `git`, and provides an `AnnexReadableFile` `Readable` that streams from the first URL that can be opened. - `LocalFileAsset.content_source` lets an asset read its content from a `Readable` instead of `filepath`; `pynwb_utils.validate()` accepts a `readable`, and `NWBAsset` runs nwbinspector on the streamed file via `inspect_nwbfile_object()`. - Each streamed file yields an INFO `DANDI.FILE_CONTENT_STREAMED` result naming the URL; a file that cannot be streamed yields `DANDI.FILE_CONTENT_MISSING`. BIDS content-dependent errors are suppressed for annexed files under `stream` as under `only-non-data`. - `dandi validate` now accepts a broken symlink directly as a path argument (`click.Path(exists=True)` used to reject it). - Docs: describe validating DataLad Dandisets, including every Dandiset of the `dandisets` superdataset, and the datalad-fuse alternative. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HFhMPs6vF5zyeHDvPTvRcA
1 parent c3931cc commit 5b292aa

12 files changed

Lines changed: 1322 additions & 52 deletions

File tree

‎dandi/cli/cmd_validate.py‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import os
88
import re
99
import sys
10-
from typing import IO, Union, cast
10+
from typing import IO, Any, Union, cast
1111
import warnings
1212

1313
import click
@@ -35,6 +35,25 @@ class TruncationNotice:
3535
"""Number of validation results omitted from this group."""
3636

3737

38+
class ExistingPath(click.Path):
39+
"""
40+
Like ``click.Path(exists=True)``, but also accepting broken symbolic links,
41+
such as annexed files whose content is not present, which ``dandi validate``
42+
knows how to handle (see ``--missing-file-content``)
43+
"""
44+
45+
def convert(
46+
self, value: Any, param: click.Parameter | None, ctx: click.Context | None
47+
) -> Any:
48+
if not os.path.lexists(value):
49+
self.fail(
50+
f"{self.name.title()} {os.fsdecode(value)!r} does not exist.",
51+
param,
52+
ctx,
53+
)
54+
return super().convert(value, param, ctx)
55+
56+
3857
STRUCTURED_FORMATS = ("json", "json_pp", "json_lines", "yaml")
3958

4059
_EXT_TO_FORMAT = {
@@ -217,8 +236,13 @@ def validate_bids(
217236
"in a datalad dataset without fetched data). 'error' (default) emits a "
218237
"concise error per file, 'skip' skips each such file with a warning, "
219238
"'only-non-data' skips content-dependent validators but still validates "
220-
"path layout.",
221-
type=click.Choice(["error", "only-non-data", "skip"], case_sensitive=True),
239+
"path layout, 'stream' streams the content of annexed files from the URLs "
240+
"registered for them in git-annex so that content-dependent validators run "
241+
"without the files having to be downloaded (requires fsspec; install with "
242+
"`pip install 'dandi[extras]'`).",
243+
type=click.Choice(
244+
["error", "only-non-data", "skip", "stream"], case_sensitive=True
245+
),
222246
default="error",
223247
)
224248
@click.option(
@@ -228,7 +252,7 @@ def validate_bids(
228252
multiple=True,
229253
default=(),
230254
)
231-
@click.argument("paths", nargs=-1, type=click.Path(exists=True, dir_okay=True))
255+
@click.argument("paths", nargs=-1, type=ExistingPath(dir_okay=True))
232256
@click.pass_context
233257
@devel_debug_option()
234258
@map_to_click_exceptions

‎dandi/cli/tests/test_cmd_validate.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,3 +996,60 @@ def test_validate_missing_file_content_no_broken_symlinks(tmp_path: Path) -> Non
996996
assert r_default.exit_code == r_skip.exit_code == 0
997997
assert "FILE_CONTENT_MISSING" not in r_default.output
998998
assert "FILE_CONTENT_MISSING" not in r_skip.output
999+
1000+
1001+
@pytest.mark.ai_generated
1002+
def test_validate_missing_file_content_stream(
1003+
tmp_path: Path, simple3_nwb: Path
1004+
) -> None:
1005+
"""--missing-file-content=stream validates annexed content via registered URLs."""
1006+
from ...tests.fixtures import annex_key_for_file, make_annexed_dandiset
1007+
from ...tests.skip import skipif
1008+
1009+
skipif.no_git()
1010+
pytest.importorskip("fsspec")
1011+
ds = tmp_path / "ds"
1012+
make_annexed_dandiset(
1013+
ds,
1014+
{
1015+
"sub-001/sub-001.nwb": (
1016+
annex_key_for_file(simple3_nwb),
1017+
[simple3_nwb.as_uri()],
1018+
)
1019+
},
1020+
)
1021+
out = tmp_path / "out.jsonl"
1022+
r = CliRunner().invoke(
1023+
validate,
1024+
[
1025+
"--missing-file-content",
1026+
"stream",
1027+
"--min-severity",
1028+
"INFO",
1029+
"-f",
1030+
"json_lines",
1031+
"-o",
1032+
str(out),
1033+
str(ds),
1034+
],
1035+
)
1036+
assert "Traceback" not in r.output
1037+
ids = [rec.id for rec in load_validation_jsonl([str(out)])]
1038+
assert "DANDI.FILE_CONTENT_STREAMED" in ids
1039+
assert "DANDI.FILE_CONTENT_MISSING" not in ids
1040+
# simple3_nwb lacks a subject_id, which only content-based checks notice
1041+
assert "NWBI.check_subject_id_exists" in ids
1042+
assert r.exit_code == 1
1043+
1044+
1045+
@pytest.mark.ai_generated
1046+
def test_validate_broken_symlink_path_argument(tmp_path: Path) -> None:
1047+
"""A broken symlink can be given directly as a path to validate."""
1048+
ds = _make_dandiset_with_broken_symlinks(tmp_path)
1049+
r = CliRunner().invoke(validate, [str(ds / "sub-001" / "sub-001.nwb")])
1050+
assert r.exit_code == 1
1051+
assert "does not exist" not in r.output
1052+
assert "FILE_CONTENT_MISSING" in r.output
1053+
r = CliRunner().invoke(validate, [str(ds / "sub-001" / "nonexistent.nwb")])
1054+
assert r.exit_code == 2
1055+
assert "does not exist" in r.output

‎dandi/files/bases.py‎

Lines changed: 73 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from pathlib import Path
1111
import re
1212
from threading import Lock
13+
import traceback
1314
from typing import IO, Any, Generic
1415
from xml.etree.ElementTree import fromstring
1516

@@ -32,7 +33,7 @@
3233
set_asset_schema_key,
3334
)
3435
from dandi.metadata.core import get_default_metadata
35-
from dandi.misctypes import DUMMY_DANDI_ETAG, Digest, LocalReadableFile, P
36+
from dandi.misctypes import DUMMY_DANDI_ETAG, Digest, LocalReadableFile, P, Readable
3637
from dandi.utils import post_upload_size_check, pre_upload_size_check, yaml_load
3738
from dandi.validate._types import (
3839
ORIGIN_INTERNAL_DANDI,
@@ -320,12 +321,22 @@ class LocalFileAsset(LocalAsset):
320321
an asset of a Dandiset
321322
"""
322323

324+
#: An alternative source for the asset's content, used instead of the file
325+
#: at `filepath` when set. This is used to stream the content of an
326+
#: annexed file (e.g., in a DataLad dataset) that is not present locally
327+
#: from a URL registered in git-annex; see `dandi.support.annex`.
328+
content_source: Readable | None = None
329+
330+
def _content(self) -> Path | Readable:
331+
"""The source to read the asset's content from"""
332+
return self.content_source if self.content_source is not None else self.filepath
333+
323334
def get_metadata(
324335
self,
325336
digest: Digest | None = None,
326337
ignore_errors: bool = True,
327338
) -> BareAsset:
328-
metadata = get_default_metadata(self.filepath, digest=digest)
339+
metadata = get_default_metadata(self._content(), digest=digest)
329340
metadata.path = self.path
330341
return metadata
331342

@@ -515,7 +526,7 @@ def get_metadata(
515526
from dandi.metadata.nwb import nwb2asset
516527

517528
try:
518-
metadata = nwb2asset(self.filepath, digest=digest)
529+
metadata = nwb2asset(self._content(), digest=digest)
519530
except Exception as e:
520531
lgr.warning(
521532
"Failed to extract NWB metadata from %s: %s: %s",
@@ -524,7 +535,7 @@ def get_metadata(
524535
str(e),
525536
)
526537
if ignore_errors:
527-
metadata = get_default_metadata(self.filepath, digest=digest)
538+
metadata = get_default_metadata(self._content(), digest=digest)
528539
else:
529540
raise
530541
metadata.path = self.path
@@ -555,12 +566,16 @@ def get_validation_errors(
555566
pass
556567
else:
557568
# Avoid heavy import by importing within function:
558-
from nwbinspector import Importance, inspect_nwbfile, load_config
569+
from nwbinspector import Importance, load_config
559570

560571
# Avoid heavy import by importing within function:
561572
from dandi.pynwb_utils import validate as pynwb_validate
562573

563-
errors.extend(pynwb_validate(self.filepath, devel_debug=devel_debug))
574+
errors.extend(
575+
pynwb_validate(
576+
self.filepath, devel_debug=devel_debug, readable=self.content_source
577+
)
578+
)
564579
if schema_version is not None:
565580
errors.extend(
566581
super().get_validation_errors(
@@ -576,9 +591,7 @@ def get_validation_errors(
576591
validator_version=str(_get_nwb_inspector_version()),
577592
)
578593

579-
for error in inspect_nwbfile(
580-
nwbfile_path=self.filepath,
581-
skip_validate=True,
594+
for error in self._inspect_nwbfile(
582595
config=load_config(filepath_or_keyword="dandi"),
583596
importance_threshold=Importance.BEST_PRACTICE_VIOLATION,
584597
):
@@ -623,6 +636,57 @@ def get_validation_errors(
623636
)
624637
return errors
625638

639+
def _inspect_nwbfile(self, **kwargs: Any) -> Iterator[Any]:
640+
"""
641+
Yield the messages from inspecting the file with nwbinspector, reading
642+
the content from `content_source` if it is set (in which case what
643+
`nwbinspector.inspect_nwbfile()` does for a local file is replicated
644+
here for the streamed content)
645+
"""
646+
# Avoid heavy import by importing within function:
647+
from nwbinspector import (
648+
Importance,
649+
InspectorMessage,
650+
inspect_nwbfile,
651+
inspect_nwbfile_object,
652+
)
653+
654+
if self.content_source is None:
655+
yield from inspect_nwbfile(
656+
nwbfile_path=self.filepath, skip_validate=True, **kwargs
657+
)
658+
return
659+
660+
# Avoid heavy import by importing within function:
661+
import h5py
662+
from pynwb import NWBHDF5IO
663+
664+
from dandi.pynwb_utils import open_readable
665+
666+
try:
667+
with open_readable(self.content_source) as fp, h5py.File(
668+
fp, "r"
669+
) as h5, NWBHDF5IO(file=h5, mode="r", load_namespaces=True) as io:
670+
nwbfile = io.read()
671+
for message in inspect_nwbfile_object(nwbfile_object=nwbfile, **kwargs):
672+
if message is not None:
673+
message.file_path = str(self.filepath)
674+
yield message
675+
except Exception as e:
676+
# Report the failure the same way inspect_nwbfile() does for a
677+
# local file that PyNWB cannot read
678+
yield InspectorMessage(
679+
message=traceback.format_exc(),
680+
importance=Importance.ERROR,
681+
check_function_name=(
682+
f"During io.read(), an error occurred: "
683+
f"{type(e).__module__}.{type(e).__name__}. "
684+
"This indicates that PyNWB was unable to read the file. "
685+
"See the traceback message for more details."
686+
),
687+
file_path=str(self.filepath),
688+
)
689+
626690

627691
class VideoAsset(LocalFileAsset):
628692
pass

‎dandi/files/bids.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ def get_metadata(
250250
) -> BareAsset:
251251
metadata = self.bids_dataset_description.get_asset_metadata(self)
252252
start_time = end_time = datetime.now().astimezone()
253-
add_common_metadata(metadata, self.filepath, start_time, end_time, digest)
253+
add_common_metadata(metadata, self._content(), start_time, end_time, digest)
254254
metadata.path = self.path
255255
return metadata
256256

‎dandi/pynwb_utils.py‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -616,8 +616,9 @@ def rename_if_matched(_name: str, obj: Any) -> None:
616616
f.visititems(rename_if_matched)
617617

618618

619-
@validate_cache.memoize_path
620-
def validate(path: str | Path, devel_debug: bool = False) -> list[ValidationResult]:
619+
def validate(
620+
path: str | Path, devel_debug: bool = False, *, readable: Readable | None = None
621+
) -> list[ValidationResult]:
621622
"""Run validation on a file and return errors
622623
623624
In case of an exception being thrown, an error message added to the
@@ -626,8 +627,35 @@ def validate(path: str | Path, devel_debug: bool = False) -> list[ValidationResu
626627
Parameters
627628
----------
628629
path: str or Path
630+
Path of the file, as reported in the returned results
631+
devel_debug: bool, optional
632+
Whether to re-raise exceptions instead of reporting them as errors
633+
readable: Readable, optional
634+
If given, the file's content is read from this `Readable` instead of from
635+
``path`` (e.g., to stream the content of an annexed file which is not
636+
present locally); the results are then not cached.
629637
"""
638+
if readable is not None:
639+
return _validate(path, readable, devel_debug=devel_debug)
640+
# The memoizing decorator hides the return type from mypy
641+
return cast(
642+
"list[ValidationResult]", _validate_cached(path, devel_debug=devel_debug)
643+
)
644+
645+
646+
@validate_cache.memoize_path
647+
def _validate_cached(
648+
path: str | Path, devel_debug: bool = False
649+
) -> list[ValidationResult]:
650+
return _validate(path, None, devel_debug=devel_debug)
651+
652+
653+
def _validate(
654+
path: str | Path, readable: Readable | None, devel_debug: bool = False
655+
) -> list[ValidationResult]:
630656
path = str(path) # Might come in as pathlib's PATH
657+
# What to read the content from:
658+
source: str | Readable = readable if readable is not None else path
631659
errors: list[ValidationResult] = []
632660

633661
# To overcome
@@ -639,7 +667,7 @@ def validate(path: str | Path, devel_debug: bool = False) -> list[ValidationResu
639667
)
640668
version = None
641669
try:
642-
version = get_nwb_version(path, sanitize=False)
670+
version = get_nwb_version(source, sanitize=False)
643671
except Exception:
644672
# we just will not remove any errors, it is required so should be some
645673
pass
@@ -685,7 +713,12 @@ def validate(path: str | Path, devel_debug: bool = False) -> list[ValidationResu
685713
)
686714

687715
try:
688-
if Version(pynwb.__version__) >= Version("3.0.0"):
716+
if readable is not None:
717+
with open_readable(readable) as fp, h5py.File(fp, "r") as h5, NWBHDF5IO(
718+
file=h5, mode="r", load_namespaces=True
719+
) as reader:
720+
error_outputs = pynwb.validate(io=reader)
721+
elif Version(pynwb.__version__) >= Version("3.0.0"):
689722
error_outputs = pynwb.validate(path=path)
690723
elif Version(pynwb.__version__) >= Version(
691724
"2.2.0"
@@ -695,6 +728,9 @@ def validate(path: str | Path, devel_debug: bool = False) -> list[ValidationResu
695728
else: # Fallback if an older version
696729
with pynwb.NWBHDF5IO(path=path, mode="r", load_namespaces=True) as reader:
697730
error_outputs = pynwb.validate(io=reader)
731+
if isinstance(error_outputs, tuple):
732+
# Older pynwb versions return (errors, status) for some call forms
733+
error_outputs = error_outputs[0]
698734
except Exception as exc:
699735
if devel_debug:
700736
raise

0 commit comments

Comments
 (0)