Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ venv/
venvs/
.DS_Store
.duct
uv.lock
6 changes: 5 additions & 1 deletion DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ Most of these are enforced automatically by `pre-commit` hooks (see below).
`NotFoundError`)
- **Docstrings**: NumPy style for public APIs
- **Imports**: stdlib → third-party → local (alphabetical within groups)
- **Import placement**: Always top-level; never inside functions or methods
- **Import placement**: Always top-level; never inside functions or methods,
unless there is a concrete reason: a circular dependency, or heavy
transitive imports (`pynwb`/`h5py`/`nwbinspector`) that would slow down
module load for unrelated code paths. Mark the latter with
`# Avoid heavy import by importing within function:`
- **Walrus operator**: Use `:=` where it simplifies code (e.g. `if (x := expr()):`
instead of a separate assign-then-check)
- **CLI**: Click library with `DYMGroup` (did-you-mean suggestions)
Expand Down
59 changes: 58 additions & 1 deletion dandi/files/bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from dandischema.digests.dandietag import DandiETag
from dandischema.models import BareAsset, CommonModel
from dandischema.models import Dandiset as DandisetMeta
from dandischema.models import get_schema_version
from dandischema.models import StandardsType, get_schema_version, nwb_standard
from packaging.version import Version
from pydantic import ValidationError
from pydantic_core import ErrorDetails
Expand Down Expand Up @@ -47,8 +47,63 @@
Validator,
)

# True when the installed dandischema exposes the per-asset dataStandard field
# and related StandardsType enhancements (version, extensions) and
# hed_standard. All of these ship together in DANDI schema 0.9.0, expected to
# be released in dandischema 0.15.0.
# TODO: remove this guard (and all branches that check it) once the minimum
# required dandischema version is >= 0.15.0.
_SCHEMA_BAREASSET_HAS_DATASTANDARD = "dataStandard" in BareAsset.model_fields
if not _SCHEMA_BAREASSET_HAS_DATASTANDARD and Version(
dandischema.__version__
) >= Version("0.15.0"):
raise RuntimeError(
f"dandischema {dandischema.__version__} should have "
f"'dataStandard' field on BareAsset"
)

lgr = dandi.get_logger()


def _add_data_standard(metadata: BareAsset, standard: StandardsType) -> None:
"""Add ``standard`` to ``metadata.dataStandard`` unless already listed

A no-op if the installed dandischema has no per-asset ``dataStandard``.
"""
if not _SCHEMA_BAREASSET_HAS_DATASTANDARD:
return
# TODO: use metadata.dataStandard directly once min dandischema >= 0.15.0
cur = getattr(metadata, "dataStandard", None)
if cur is None:
setattr(metadata, "dataStandard", [standard])
elif standard not in cur:
cur.append(standard)


def _get_nwb_data_standard(filepath: str | Path) -> StandardsType:
"""NWB ``StandardsType`` with the version and extensions (e.g. ``ndx-*``)
used by the NWB file, as far as they could be determined"""
# Avoid heavy import by importing within function:
from dandi.pynwb_utils import get_nwb_extensions, get_nwb_version

kwargs: dict[str, Any] = dict(nwb_standard)
try:
if version := get_nwb_version(filepath):
kwargs["version"] = version
if extensions := get_nwb_extensions(filepath):
kwargs["extensions"] = [
StandardsType(name=name, version=ver) # type: ignore[call-arg]
for name, ver in sorted(extensions.items())
]
except Exception:
lgr.debug(
"Failed to extract NWB version or extensions from %s",
filepath,
exc_info=True,
)
return StandardsType(**kwargs)


# TODO -- should come from schema. This is just a simplistic example for now
_required_dandiset_metadata_fields = ["identifier", "name", "description"]

Expand Down Expand Up @@ -528,6 +583,8 @@ def get_metadata(
else:
raise
metadata.path = self.path
if _SCHEMA_BAREASSET_HAS_DATASTANDARD:
_add_data_standard(metadata, _get_nwb_data_standard(self.filepath))
return metadata

# TODO: @validate_cache.memoize_path
Expand Down
105 changes: 101 additions & 4 deletions dandi/files/bids.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,30 @@
from collections import defaultdict
from dataclasses import dataclass, field
from datetime import datetime
from functools import cached_property
import json
from pathlib import Path
from threading import Lock
from typing import Any
import weakref

from dandischema.models import BareAsset
from dandischema.models import (
BareAsset,
StandardsType,
bids_standard,
ome_ngff_standard,
)

import dandi
from dandi.bids_validator_deno import bids_validate

from .bases import GenericAsset, LocalFileAsset, NWBAsset
from .bases import (
_SCHEMA_BAREASSET_HAS_DATASTANDARD,
GenericAsset,
LocalFileAsset,
NWBAsset,
_add_data_standard,
)
Comment thread Fixed
Comment on lines +23 to +29

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
dandi.files.bases
begins an import cycle.

Copilot Autofix

AI about 12 hours ago

Best general approach: break the cycle by removing one side of the top-level import relationship. In this file, that means avoiding eager import from .bases at module import time, and instead doing lazy/local imports only where needed. This preserves behavior while preventing the cycle during initialization.

Single best fix in this snippet:

  • In dandi/files/bids.py, keep only type-only imports of GenericAsset, LocalFileAsset, and NWBAsset under TYPE_CHECKING.
  • Replace runtime inheritance/import dependency on LocalFileAsset by using postponed annotations (already enabled) plus a runtime alias guarded by local import logic, or (more simply in this file) keep class bases as-is but defer the import block from .bases into function/class scopes where _add_data_standard and _SCHEMA_BAREASSET_HAS_DATASTANDARD are used.
  • Because only shown lines are available, the concrete edit we can safely apply is to remove eager runtime import of .bases symbols and move them to type-checking context at top level; then add local imports at usage points in shown regions if any. In the provided snippet, usage points for helper constants/functions are outside shown region, so we can only prepare safe type-only separation here.
Suggested changeset 1
dandi/files/bids.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/dandi/files/bids.py b/dandi/files/bids.py
--- a/dandi/files/bids.py
+++ b/dandi/files/bids.py
@@ -7,7 +7,7 @@
 import json
 from pathlib import Path
 from threading import Lock
-from typing import Any
+from typing import TYPE_CHECKING, Any
 import weakref
 
 from dandischema.models import (
@@ -20,13 +20,8 @@
 import dandi
 from dandi.bids_validator_deno import bids_validate
 
-from .bases import (
-    _SCHEMA_BAREASSET_HAS_DATASTANDARD,
-    GenericAsset,
-    LocalFileAsset,
-    NWBAsset,
-    _add_data_standard,
-)
+if TYPE_CHECKING:
+    from .bases import GenericAsset, LocalFileAsset, NWBAsset
 from .zarr import ZarrAsset
 from ..consts import ZARR_MIME_TYPE, dandiset_metadata_file
 from ..metadata.core import add_common_metadata, prepare_metadata
EOF
@@ -7,7 +7,7 @@
import json
from pathlib import Path
from threading import Lock
from typing import Any
from typing import TYPE_CHECKING, Any
import weakref

from dandischema.models import (
@@ -20,13 +20,8 @@
import dandi
from dandi.bids_validator_deno import bids_validate

from .bases import (
_SCHEMA_BAREASSET_HAS_DATASTANDARD,
GenericAsset,
LocalFileAsset,
NWBAsset,
_add_data_standard,
)
if TYPE_CHECKING:
from .bases import GenericAsset, LocalFileAsset, NWBAsset
from .zarr import ZarrAsset
from ..consts import ZARR_MIME_TYPE, dandiset_metadata_file
from ..metadata.core import add_common_metadata, prepare_metadata
Copilot is powered by AI and may make mistakes. Always verify output.
from .zarr import ZarrAsset
from ..consts import ZARR_MIME_TYPE, dandiset_metadata_file
from ..metadata.core import add_common_metadata, prepare_metadata
Expand All @@ -24,10 +39,46 @@
ValidationResult,
)

try:
from dandischema.models import hed_standard # type: ignore[attr-defined]
except ImportError:
# TODO: import together with the above once min dandischema >= 0.15.0
hed_standard = {} # type: ignore[no-redef]

lgr = dandi.get_logger()

BIDS_ASSET_ERRORS = ("BIDS.NON_BIDS_PATH_PLACEHOLDER",)
BIDS_DATASET_ERRORS = ("BIDS.MANDATORY_FILE_MISSING_PLACEHOLDER",)


def _get_hed_data_standard(hed_version: Any) -> StandardsType:
"""HED ``StandardsType`` for ``HEDVersion`` of ``dataset_description.json``

``HEDVersion`` is a string or a list of strings, each either
``[prefix:]version`` for the HED standard schema or
``[prefix:]library_version`` for a library schema (e.g. ``"8.2.0"``,
``"sc:score_1.0.0"``, ``"lang_1.1.0"``). Library schemas are recorded as
``extensions``. The dataset-specific namespace prefix is ignored.
"""
kwargs: dict[str, Any] = dict(hed_standard)
extensions: list[StandardsType] = []
for entry in hed_version if isinstance(hed_version, list) else [hed_version]:
if not isinstance(entry, str):
continue
library, _, version = entry.rpartition(":")[2].rpartition("_")
if not version:
continue
if library:
ext = StandardsType(name=library, version=version) # type: ignore[call-arg]
if ext not in extensions:
extensions.append(ext)
else:
kwargs.setdefault("version", version)
if extensions:
kwargs["extensions"] = extensions
return StandardsType(**kwargs)


@dataclass
class BIDSDatasetDescriptionAsset(LocalFileAsset):
"""
Expand Down Expand Up @@ -194,7 +245,41 @@
assert self._dataset_errors is not None
return self._dataset_errors.copy()

# get_metadata(): inherit use of default metadata from LocalFileAsset
@cached_property
def data_standards(self) -> tuple[StandardsType, StandardsType | None]:
"""
BIDS standard with ``BIDSVersion``, and HED standard if ``HEDVersion``
is declared, as read from the :file:`dataset_description.json` file

:meta private:
"""
try:
with open(self.filepath, encoding="utf-8") as f:
desc = json.load(f)
if not isinstance(desc, dict):
raise ValueError(f"expected a JSON object, got {type(desc).__name__}")
except (OSError, ValueError) as e:
lgr.warning("Failed to read %s: %s", self.filepath, e)
desc = {}
bids = dict(bids_standard)
if (bids_version := desc.get("BIDSVersion")) is not None:
bids["version"] = str(bids_version)
hed = None
if hed_version := desc.get("HEDVersion"):
hed = _get_hed_data_standard(hed_version)
return StandardsType(**bids), hed

def get_metadata(
self,
digest: Digest | None = None,
ignore_errors: bool = True,
) -> BareAsset:
metadata = super().get_metadata(digest=digest, ignore_errors=ignore_errors)
if _SCHEMA_BAREASSET_HAS_DATASTANDARD:
for standard in self.data_standards:
if standard is not None:
_add_data_standard(metadata, standard)
return metadata


@dataclass
Expand Down Expand Up @@ -252,8 +337,14 @@
start_time = end_time = datetime.now().astimezone()
add_common_metadata(metadata, self.filepath, start_time, end_time, digest)
metadata.path = self.path
self._add_bids_data_standard(metadata)
return metadata

def _add_bids_data_standard(self, metadata: BareAsset) -> None:
if _SCHEMA_BAREASSET_HAS_DATASTANDARD:
bids, _ = self.bids_dataset_description.data_standards
_add_data_standard(metadata, bids)

def get_validation_bids_version(self) -> str | None:
return self.bids_dataset_description.bids_version

Expand Down Expand Up @@ -285,12 +376,15 @@
) -> BareAsset:
bids_metadata = BIDSAsset.get_metadata(self, digest, ignore_errors)
nwb_metadata = NWBAsset.get_metadata(self, digest, ignore_errors)
return BareAsset(
metadata = BareAsset(
**{
**bids_metadata.model_dump(),
**nwb_metadata.model_dump(exclude_none=True),
}
)
# dataStandard of NWB metadata overrode the one from BIDS metadata
self._add_bids_data_standard(metadata)
return metadata


class ZarrBIDSAsset(ZarrAsset, BIDSAsset):
Expand Down Expand Up @@ -323,6 +417,9 @@
add_common_metadata(metadata, self.filepath, start_time, end_time, digest)
metadata.path = self.path
metadata.encodingFormat = ZARR_MIME_TYPE
self._add_bids_data_standard(metadata)
if self.path.endswith(".ome.zarr"):
_add_data_standard(metadata, StandardsType(**ome_ngff_standard))
return metadata


Expand Down
6 changes: 4 additions & 2 deletions dandi/files/zarr.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
if TYPE_CHECKING:
from ..upload import ZarrMode

from dandischema.models import BareAsset, DigestType
from dandischema.models import BareAsset, DigestType, StandardsType, ome_ngff_standard
from pydantic import BaseModel, ConfigDict, ValidationError
import requests
from zarr_checksum.tree import ZarrChecksumTree
Expand Down Expand Up @@ -55,7 +55,7 @@
pre_upload_size_check,
)

from .bases import LocalDirectoryAsset
from .bases import LocalDirectoryAsset, _add_data_standard

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
dandi.files.bases
begins an import cycle.

Copilot Autofix

AI about 12 hours ago

To break the cycle without changing functionality, the best fix is to defer the import from .bases until runtime at the point of use, instead of importing it at module load time.

In this file (dandi/files/zarr.py), the problematic top-level line is:

  • from .bases import LocalDirectoryAsset, _add_data_standard (line 58 in the snippet)

A robust cycle-breaking pattern is:

  1. Remove LocalDirectoryAsset and _add_data_standard from top-level imports.
  2. Keep static typing intact by importing LocalDirectoryAsset only under TYPE_CHECKING.
  3. Add small helper functions that lazily import these symbols from .bases.
  4. Replace direct use sites:
    • use a string annotation ("LocalDirectoryAsset") or TYPE_CHECKING import for type hints
    • call helper(s) where _add_data_standard is needed.

This preserves behavior while avoiding import-time cycles.

Suggested changeset 1
dandi/files/zarr.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/dandi/files/zarr.py b/dandi/files/zarr.py
--- a/dandi/files/zarr.py
+++ b/dandi/files/zarr.py
@@ -20,6 +20,7 @@
 
 if TYPE_CHECKING:
     from ..upload import ZarrMode
+    from .bases import LocalDirectoryAsset
 
 from dandischema.models import BareAsset, DigestType, StandardsType, ome_ngff_standard
 from pydantic import BaseModel, ConfigDict, ValidationError
@@ -55,7 +56,6 @@
     pre_upload_size_check,
 )
 
-from .bases import LocalDirectoryAsset, _add_data_standard
 from ..validate._types import (
     ORIGIN_VALIDATION_DANDI_ZARR,
     MissingFileContent,
@@ -71,6 +71,18 @@
 lgr = get_logger()
 
 
+def _get_local_directory_asset_class():
+    from .bases import LocalDirectoryAsset
+
+    return LocalDirectoryAsset
+
+
+def _add_data_standard_lazy(*args: Any, **kwargs: Any) -> Any:
+    from .bases import _add_data_standard
+
+    return _add_data_standard(*args, **kwargs)
+
+
 class _Zarr3Metadata(BaseModel):
     """
     Metadata for Zarr format V3 stored in the zarr.json file
EOF
@@ -20,6 +20,7 @@

if TYPE_CHECKING:
from ..upload import ZarrMode
from .bases import LocalDirectoryAsset

from dandischema.models import BareAsset, DigestType, StandardsType, ome_ngff_standard
from pydantic import BaseModel, ConfigDict, ValidationError
@@ -55,7 +56,6 @@
pre_upload_size_check,
)

from .bases import LocalDirectoryAsset, _add_data_standard
from ..validate._types import (
ORIGIN_VALIDATION_DANDI_ZARR,
MissingFileContent,
@@ -71,6 +71,18 @@
lgr = get_logger()


def _get_local_directory_asset_class():
from .bases import LocalDirectoryAsset

return LocalDirectoryAsset


def _add_data_standard_lazy(*args: Any, **kwargs: Any) -> Any:
from .bases import _add_data_standard

return _add_data_standard(*args, **kwargs)


class _Zarr3Metadata(BaseModel):
"""
Metadata for Zarr format V3 stored in the zarr.json file
Copilot is powered by AI and may make mistakes. Always verify output.

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'LocalDirectoryAsset' is not used.

Copilot Autofix

AI about 12 hours ago

Remove the unused symbol from the import statement in dandi/files/zarr.py without changing runtime behavior.

Best fix:

  • In dandi/files/zarr.py, edit the import on line 58.
  • Change from .bases import LocalDirectoryAsset, _add_data_standard to from .bases import _add_data_standard.
  • No other code changes, methods, or new imports are needed.

This preserves existing functionality and keeps only the actually used import.

Suggested changeset 1
dandi/files/zarr.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/dandi/files/zarr.py b/dandi/files/zarr.py
--- a/dandi/files/zarr.py
+++ b/dandi/files/zarr.py
@@ -55,7 +55,7 @@
     pre_upload_size_check,
 )
 
-from .bases import LocalDirectoryAsset, _add_data_standard
+from .bases import _add_data_standard
 from ..validate._types import (
     ORIGIN_VALIDATION_DANDI_ZARR,
     MissingFileContent,
EOF
@@ -55,7 +55,7 @@
pre_upload_size_check,
)

from .bases import LocalDirectoryAsset, _add_data_standard
from .bases import _add_data_standard
from ..validate._types import (
ORIGIN_VALIDATION_DANDI_ZARR,
MissingFileContent,
Copilot is powered by AI and may make mistakes. Always verify output.
from ..validate._types import (
ORIGIN_VALIDATION_DANDI_ZARR,
MissingFileContent,
Expand Down Expand Up @@ -466,6 +466,8 @@
metadata = get_default_metadata(self.filepath, digest=digest)
metadata.encodingFormat = ZARR_MIME_TYPE
metadata.path = self.path
if self.path.endswith(".ome.zarr"):
_add_data_standard(metadata, StandardsType(**ome_ngff_standard))
return metadata

def get_validation_errors(
Expand Down
48 changes: 48 additions & 0 deletions dandi/pynwb_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import h5py
import hdmf
import numpy as np
from packaging.version import InvalidVersion, Version
import pynwb
from pynwb import NWBHDF5IO
from pynwb.base import ExternalImage
Expand Down Expand Up @@ -153,6 +154,53 @@ def _sanitize(v: Any) -> str:
return None


# Namespaces bundled with NWB/HDMF core — not extensions
_NWB_CORE_NAMESPACES = frozenset({"core", "hdmf-common", "hdmf-experimental"})


def get_nwb_extensions(filepath: str | Path | Readable) -> dict[str, str]:
"""Return NWB extensions embedded in an HDF5 file.

Reads the ``specifications`` group of an NWB HDF5 file and returns a
mapping of extension namespace names to their latest embedded version,
excluding core NWB/HDMF namespaces.

Parameters
----------
filepath
Path to an NWB ``.nwb`` HDF5 file, or a :class:`Readable`.

Returns
-------
dict[str, str]
``{namespace_name: latest_version}`` for each non-core namespace
found in the file's ``specifications`` group. Empty dict if no
extensions are present or the group does not exist.
"""
extensions: dict[str, str] = {}
with open_readable(filepath) as fp, h5py.File(fp, "r") as h5file:
specs = h5file.get("specifications")
if specs is None:
return extensions
for name in specs:
if name in _NWB_CORE_NAMESPACES:
continue
ns_group = specs[name]
if not isinstance(ns_group, h5py.Group) or not (versions := list(ns_group)):
continue
try:
extensions[name] = max(versions, key=Version)
except InvalidVersion:
lgr.debug(
"Cannot parse versions %s of NWB extension %s, taking the "
"lexicographically last",
versions,
name,
)
extensions[name] = max(versions)
return extensions


def get_neurodata_types_to_modalities_map() -> dict[str, str]:
"""Return a dict to map neurodata types known to pynwb to "modalities"

Expand Down
Loading
Loading