Repository navigation
Populate per-asset dataStandard for BIDS, HED, NWB, and extensions - #1811
yarikoptic wants to merge 10 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1811 +/- ##
==========================================
+ Coverage 90.78% 90.96% +0.18%
==========================================
Files 92 92
Lines 14130 14382 +252
==========================================
+ Hits 12828 13083 +255
+ Misses 1302 1299 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| BIDS_DATASET_ERRORS = ("BIDS.MANDATORY_FILE_MISSING_PLACEHOLDER",) | ||
|
|
||
|
|
||
| def _add_standard( |
There was a problem hiding this comment.
line 542 of bases appears to have similar logic
|
@yarikoptic lgtm! Let me just check on some of my own data |
|
Added NWB spec version extraction in 279c297.
Tested against dandiset 001636 (346 NWB files, macaque electrophysiology): Before: After: |
bendichter
left a comment
There was a problem hiding this comment.
ok, I made a minor correction. LGTM now!
- BIDSDatasetDescriptionAsset.get_metadata(): always sets BIDS standard (with BIDSVersion), adds HED when HEDVersion present in JSON, warns on read failure - NWBAsset.get_metadata(): sets NWB standard - ZarrBIDSAsset.get_metadata(): sets OME/NGFF for .ome.zarr assets - Guard for older dandischema without dataStandard on BareAsset; RuntimeError if dandischema >= 0.12.2 lacks it - Register ai_generated pytest marker in tox.ini Requires dandischema with per-asset dataStandard support (0.12.2+). Works silently with older dandischema. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add get_nwb_extensions() to pynwb_utils that reads h5file["specifications"] to discover ndx-* namespaces and their versions (filtering out core/hdmf) - NWBAsset.get_metadata() populates StandardsType.extensions with ndx-* extensions found in the NWB file - BIDSDatasetDescriptionAsset.get_metadata() extracts HED library schemas from list-valued HEDVersion (e.g. ["8.2.0", "sc:1.0.0"]) into extensions - Add tests for both NWB extension extraction and HED library schema parsing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…S_DATASTANDARD Single bool in bases.py guards all dataStandard/version/extensions features that ship together in dandischema >= 0.12.2, replacing scattered "field in Model.model_fields" checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…PMENT.md Move json, h5py, get_nwb_extensions, and _SCHEMA_BAREASSET_HAS_DATASTANDARD imports to module top level in tests. Keep pynwb_utils import deferred in bases.py (heavy transitive deps: h5py/pynwb/hdmf/numpy) per existing convention. Add import guidance to DEVELOPMENT.md (CLAUDE.md defers to it). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
hed_standard does not exist in dandischema < 0.12.2, so gate the import on _SCHEMA_BAREASSET_HAS_DATASTANDARD (all new symbols ship together). HED detection is skipped when unavailable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move _SCHEMA_BAREASSET_HAS_DATASTANDARD guard block after all imports to fix E402. Use getattr/setattr for dataStandard access and type: ignore[call-arg] for StandardsType(version=...) to satisfy mypy against released dandischema that lacks these fields. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use the existing get_nwb_version() to populate the version field on the NWB StandardsType entry so per-asset metadata reports the NWB spec version (e.g. 2.9.0) alongside extensions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Factor out _add_data_standard() and _get_nwb_data_standard() into files/bases.py and use them from NWB, BIDS and Zarr assets, instead of the duplicated append-if-missing logic (review by @bendichter). - NWB: a file whose version/extensions cannot be read no longer makes get_metadata(ignore_errors=True) raise; it gets a bare NWB standard. - get_nwb_extensions(): import packaging's Version (it was no longer imported after rebase, and the broad `except Exception` silently dropped every extension), catch only InvalidVersion, and fall back to the lexicographically last version instead of dropping the extension. - HED: parse HEDVersion entries per HED spec as `[prefix:]version` or `[prefix:]library_version` (e.g. "sc:score_1.0.0", partnered "lang_1.1.0"), recording libraries as extensions named by library, not by the dataset-specific prefix. Ignore non-string entries. - BIDS: declare BIDS (with BIDSVersion) on every asset of a BIDS dataset, not only dataset_description.json, as dataStandard lists the standards applicable to the asset; keep it for NWB files in BIDS datasets, whose merged metadata had the NWB dataStandard override the BIDS one. Read dataset_description.json once, as UTF-8, and do not crash on non-string BIDSVersion or a file which is not a JSON object. - OME/NGFF: also declare it for non-BIDS .ome.zarr assets, detected by the ".ome.zarr" ending. - Expect the feature in dandischema 0.15.0 (DANDI schema 0.9.0). - Tests: cover the above, incl. NWBAsset with an embedded extension, a BIDS dataset with NWB/Zarr/other assets, and dropping of per-asset dataStandard by _maybe_downgrade_metadata() for 0.7.0/0.8.0 servers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P3VJkUsu17gjmUEf4ZY1RW
This enables CI to test against the unreleased dandischema that has the dataStandard field and StandardsType enhancements. TO BE REVERTED before merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| from .bases import ( | ||
| _SCHEMA_BAREASSET_HAS_DATASTANDARD, | ||
| GenericAsset, | ||
| LocalFileAsset, | ||
| NWBAsset, | ||
| _add_data_standard, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 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 ofGenericAsset,LocalFileAsset, andNWBAssetunderTYPE_CHECKING. - Replace runtime inheritance/import dependency on
LocalFileAssetby 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.basesinto function/class scopes where_add_data_standardand_SCHEMA_BAREASSET_HAS_DATASTANDARDare used. - Because only shown lines are available, the concrete edit we can safely apply is to remove eager runtime import of
.basessymbols 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.
| @@ -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 |
| ) | ||
|
|
||
| from .bases import LocalDirectoryAsset | ||
| from .bases import LocalDirectoryAsset, _add_data_standard |
Check notice
Code scanning / CodeQL
Cyclic import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 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:
- Remove
LocalDirectoryAssetand_add_data_standardfrom top-level imports. - Keep static typing intact by importing
LocalDirectoryAssetonly underTYPE_CHECKING. - Add small helper functions that lazily import these symbols from
.bases. - Replace direct use sites:
- use a string annotation (
"LocalDirectoryAsset") or TYPE_CHECKING import for type hints - call helper(s) where
_add_data_standardis needed.
- use a string annotation (
This preserves behavior while avoiding import-time cycles.
| @@ -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 |
| ) | ||
|
|
||
| from .bases import LocalDirectoryAsset | ||
| from .bases import LocalDirectoryAsset, _add_data_standard |
Check notice
Code scanning / CodeQL
Unused import Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 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_standardtofrom .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.
| @@ -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, |
Populates the per-asset
dataStandardintroduced in dandi/dandi-schema#371 so the archive can tell which standards, versions and extensions each asset (and thus dandiset) uses:ndx-*extensions (latest cached version of each) from the HDF5specificationsgroup (get_nwb_extensions()).BIDSVersionfromdataset_description.json, declared on every asset of the BIDS dataset.HEDVersion, ondataset_description.json; library schemas ([prefix:]library_version, e.g.sc:score_1.0.0) becomeextensions..ome.zarrassets.All of it is gated on
_SCHEMA_BAREASSET_HAS_DATASTANDARD, so it is a no-op with released dandischema (< 0.15.0). The last commit (TEMP) installs dandischema from the dandi-schema PR branch and is to be replaced with a version bound once that is released.Details: review responses, changes, testing
Review responses
enh-hed(dandi-schema's own CI runs dandi-cli master against its branch)._add_data_standard()infiles/bases.py, used by NWB, BIDS and Zarr assets; NWB extraction moved into_get_nwb_data_standard().ndx-hedextension for now (as seen on 001636); anything more specific waits until the representation settles.files/bids.py:bids.pyimported from.basesalready on master, so this is not new.Changes in this round (rebased onto current master)
.gitignore,CLAUDE.md(now defers toDEVELOPMENT.md; the import-placement guidance went there),pyproject.toml, tests.get_nwb_extensions(): after rebaseVersionwas no longer imported inpynwb_utils, and the broadexcept Exceptionsilently dropped every extension. Now imports it, catches onlyInvalidVersion, and falls back to the lexicographically last version instead of dropping the extension."sc:1.0.0", which is not the spec'd format: entries are[prefix:]versionor[prefix:]library_version; the prefix is a dataset-local alias, so extensions are now named by library (score, notsc), and prefix-less partnered libraries (lang_1.1.0) work.BIDSVersion, or on adataset_description.jsonthat is not a JSON object; NWB files whose version/extensions cannot be read still get a (bare) NWB standard withignore_errors=True..ome.zarrassets (detected by the.ome.zarrending, so dots in the name are fine).dataStandardoverride the BIDS one.dataset_description.jsonis read once, as UTF-8._maybe_downgrade_metadata()drops a populated per-assetdataStandardfor 0.7.0/0.8.0 servers (needs the downgrade in Add per-asset dataStandard, HED standard, and extensions to StandardsType dandi-schema#371).pyproject.toml: when restoring the version bound after the dandischema release, set the version checked by the_SCHEMA_BAREASSET_HAS_DATASTANDARDguard (assumed 0.15.0) to that release's.Testing
ndx-hed/ndx-pose, a.ome.zarr, and bids-exampleseeg_ds003645s_hed_library:Valid(3 NWB/Zarr, 32 BIDS)ValidschemaVersiondataStandardndx-hed0.2.0 (+ndx-pose0.4.0); OME/NGFF; BIDS 1.11.1 on every BIDS asset; HED 8.4.0 +score1.0.0,testlib1.0.2 ondataset_description.jsonassetsSummary.dataStandardndx-hed,ndx-pose}; BIDS 1.11.1, HED 8.4.0 + {score,testlib} -- one entry per standard(Images from mirror.gcr.io; the 0.9.0 server is the stock
dandiarchive/dandiarchive-apiwith the dandischema wheel from the schema PR branch installed, as dandi-schema'stools/api-with-schema.Dockerfiledoes. The summary is aggregated only fromValidassets when the draft becomes pending, so the test touched the dandiset metadata after validation to get it re-aggregated.)🤖 Generated with Claude Code
https://claude.ai/code/session_01P3VJkUsu17gjmUEf4ZY1RW