Add BIDSAsset.get_bids_metadata() to get only the BIDS metadata of an asset

Getting the metadata of an NWB file in a BIDS dataset recursed because
of a cycle between two methods that each merge BIDS and NWB metadata:
`NWBBIDSAsset.get_metadata()` extracts the NWB metadata with
`dandi.metadata.nwb.get_metadata()`, which itself got the BIDS metadata
of the file with `get_metadata()` of its `NWBBIDSAsset`.  Calling
`BIDSAsset.get_metadata()` unbound only patched this one call site.

Instead, give BIDS assets a method returning only their BIDS metadata,
which no subclass overrides to read the content, and use it wherever
only the BIDS metadata is wanted: when extracting the metadata of an
NWB file, and in `NWBBIDSAsset.get_metadata()` and
`ZarrBIDSAsset.get_metadata()` (no change in behavior).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qn5WBSiQgoZoL4fytF6nEr

Author: claude

dandi/files/bids.py

@@ -248,6 +248,18 @@ def get_metadata(
         digest: Digest | None = None,
         ignore_errors: bool = True,
     ) -> BareAsset:
+        return self.get_bids_metadata(digest)
+
+    def get_bids_metadata(self, digest: Digest | None = None) -> BareAsset:
+        """
+        Return the metadata of the asset that comes from the BIDS dataset (its
+        path and files such as :file:`participants.tsv`) only.
+
+        Unlike `get_metadata()`, which subclasses override to also extract
+        metadata from the content of the asset (e.g., `NWBBIDSAsset` from the
+        NWB file), this never reads the content, so that the extraction of
+        metadata from the content can use it without recursing.
+        """
         metadata = self.bids_dataset_description.get_asset_metadata(self)
         start_time = end_time = datetime.now().astimezone()
         add_common_metadata(metadata, self.filepath, start_time, end_time, digest)
@@ -283,7 +295,7 @@ def get_metadata(
         digest: Digest | None = None,
         ignore_errors: bool = True,
     ) -> BareAsset:
-        bids_metadata = BIDSAsset.get_metadata(self, digest, ignore_errors)
+        bids_metadata = self.get_bids_metadata(digest)
         nwb_metadata = NWBAsset.get_metadata(self, digest, ignore_errors)
         return BareAsset(
             **{
@@ -318,10 +330,7 @@ def get_metadata(
         digest: Digest | None = None,
         ignore_errors: bool = True,
     ) -> BareAsset:
-        metadata = self.bids_dataset_description.get_asset_metadata(self)
-        start_time = end_time = datetime.now().astimezone()
-        add_common_metadata(metadata, self.filepath, start_time, end_time, digest)
-        metadata.path = self.path
+        metadata = self.get_bids_metadata(digest)
         metadata.encodingFormat = ZARR_MIME_TYPE
         return metadata
 

dandi/metadata/nwb.py

@@ -69,10 +69,9 @@ def get_metadata(
             assert isinstance(df, bids.BIDSAsset)
             if not digest:
                 digest = DUMMY_DANDI_ETAG
-            # Only the BIDS metadata: `df` is an `NWBBIDSAsset`, whose own
-            # `get_metadata()` would extract the NWB metadata by calling this
-            # function again, recursing until `RecursionError`
-            path_metadata = bids.BIDSAsset.get_metadata(df, digest=digest)
+            # Not `df.get_metadata()`, which for an NWB file would extract
+            # its NWB metadata, i.e., call this function again
+            path_metadata = df.get_bids_metadata(digest=digest)
             meta["bids_version"] = df.get_validation_bids_version()
             # there might be a more elegant way to do this:
             if path_metadata.wasAttributedTo:

dandi/tests/test_metadata.py

@@ -43,8 +43,9 @@
 from .fixtures import SampleDandiset
 from .skip import mark
 from .. import __version__
-from ..consts import metadata_nwb_subject_fields
+from ..consts import dandiset_metadata_file, metadata_nwb_subject_fields
 from ..dandiapi import RemoteBlobAsset
+from ..files import NWBBIDSAsset, dandi_file, find_bids_dataset_description
 from ..metadata import nwb as metadata_nwb
 from ..metadata.core import prepare_metadata
 from ..metadata.nwb import get_metadata, nwb2asset
@@ -149,6 +150,36 @@ def test_bids_nwb_metadata_no_recursion(
     assert metadata["sex"] == "U"
 
 
+@pytest.mark.ai_generated
+def test_nwb_bids_asset_metadata_no_recursion(
+    bids_examples: Path, tmp_path: Path, mocker: MockerFixture
+) -> None:
+    # The metadata of an NWBBIDSAsset merges its BIDS and NWB metadata,
+    # extracting the latter (which itself includes BIDS metadata) only once
+    dpath = tmp_path / "ieeg_epilepsyNWB"
+    shutil.copytree(bids_examples / "ieeg_epilepsyNWB", dpath)
+    (dpath / dandiset_metadata_file).write_text("identifier: '000027'\n")
+    file_path = (
+        dpath
+        / "sub-01"
+        / "ses-postimp"
+        / "ieeg"
+        / "sub-01_ses-postimp_task-seizure_run-01_ieeg.nwb"
+    )
+    # (The asset refers to it only weakly)
+    bids_dataset_description = find_bids_dataset_description(file_path)
+    df = dandi_file(file_path, dpath, bids_dataset_description=bids_dataset_description)
+    assert isinstance(df, NWBBIDSAsset)
+    spy = mocker.spy(metadata_nwb, "nwb2asset")
+    metadata = df.get_metadata(digest=DUMMY_DANDI_ETAG, ignore_errors=False)
+    assert spy.call_count == 1
+    assert metadata.wasAttributedTo is not None
+    participant = metadata.wasAttributedTo[0]
+    assert participant.identifier == "01"
+    assert participant.sex is not None
+    assert participant.sex.name == "Unknown"
+
+
 @pytest.mark.parametrize(
     "age,duration",
     [