Repository navigation
Update schema to NWB 2.11 - #300
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the embedded, auto-generated NWB/HDMF schema specifications used by AqNWB to align with the latest upstream releases referenced in Issue #298.
Changes:
- Updated the NWB core schema spec to version 2.10.0, including new/updated schema components (e.g.,
nwb.eventand updates undernwb.file,nwb.misc,nwb.image,nwb.behavior). - Updated the HDMF common schema spec to version 1.9.0, including adding the
resourcesspec (HERD external resources tables) underhdmf-common. - Updated the HDMF experimental schema spec to version 0.6.0, adjusting the namespace contents accordingly.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/spec/core.hpp | Bumps core schema to NWB 2.10.0 and incorporates new/updated core spec content (including nwb.event and updated NWBFile structure). |
| src/spec/hdmf_common.hpp | Bumps HDMF common schema to 1.9.0 and adds the resources specification (HERD) to the namespace. |
| src/spec/hdmf_experimental.hpp | Bumps HDMF experimental schema to 0.6.0 and updates namespace schema sources accordingly. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #300 +/- ##
==========================================
- Coverage 84.69% 83.66% -1.03%
==========================================
Files 57 71 +14
Lines 2699 4280 +1581
Branches 347 624 +277
==========================================
+ Hits 2286 3581 +1295
- Misses 413 699 +286 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…OMMON, CORE namespace on includes
…ject-required-by-default Make Subject required by default in NWBFile::initialize()
Add time intervals
There was a problem hiding this comment.
🟡 Changes recommended
Ragged-table reopening and ID generation can corrupt tables, while DataSpec validation permits schema-invalid datasets.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 87/88 changed files
- Comments generated: 17
- Review effort level: Balanced
…o dtype and shape in addition to name
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Fix #298
Fix #306
Update schema version to the latest releases
spec/core.hppto NWB schema2.102.11spec/hdmf_common.hppto schema version1.91.10MeaningsTable.targetis being changed to an object reference attribute in hdmf-common 1.10 and should be changed here accordingly before merge. See Change MeaningsTable.target from a link to an object-reference attribute hdmf-dev/hdmf-common-schema#97. Fix via 79b6fbf here and also updated on Add EventTable type #304 to fix the actualMeaningsTableclass.spec/hdmf_experimental.hppto schema version 0.6From #301 : Add event VectorData types
Fix #299
schematype_to_aqnwb.pyto facilitate generation the new typesresources/utilsto ensure use of the latest versions of PyNWB with uvgetIOinstead of the previousm_ioshared pointerTimestampVectorDataandDurationVectorDatatypes:nwb/event/TimestampVectorData.hpp/cppnwb/event/DurationVectorData.hpp/cppTimestampVectorDataDurationVectorDataDynamicTableproperly supports the new column types:DynamicTable.readColumnusing type traits to to support reading of both: i) VectorData columns with a specific data type and ii) columns that are a subtype of VectorData (e.g.,TimestampVectorDataandDurationVectorData)DyanmicTable.addColumnto allow adding of fully configured VectorData columns and derived column typesTimestampVectorDataandDurationVectorDatacan be added viaDynamicTable.addColumnand read back viaDynamicTable.readColumnFrom #302 : Add MeaningsTable
Fix #284
Implement new
MeaningsTabletype:src/nwb/hdmf/table/MeaningsTable.*source forMeaningsTableMeaningsTableUpdate
DynamicTableto support addingMeaningsTables:readMeaningsTablefor readingMeaningsTablesassociated with aDynamicTable. This also createscreateMeaningsTableInstancemethods for creatin a newMeaningsTablesthat is not yet initialized.DynamicTable.createMeaningsTablemethods for creating a fully initializedMeaningsTablethat is associated with a particular column of theDynamicTableDynamicTable.createMeaningsTableandDynamicTable.readMeaningsTablemethods.- [ ] Update the docs/tutorials as necessary to describe the use of MeaningsTable alongside DynamicTableThis will be part of follow-up once the full stack for EventsTable has been addedOther Changes
ElectrodesTable.initializeto add a new optional parameterrowChunkSize. Similar to theMeaningsTablethe existingElectrodesTablealso creates pre-defined columns ininitialize. Currently the chunking was set inElectrodesTableto1, which is too small. AddingrowChunkSizeincreases the default to 100 and allows the caller to set the chunk size.From #304 : Add EventsTable type
Fix #285
Add support for
EventsTableEventTabletypeEventTabletypeevents/group and addedNWBFile::createEventsTableconvenience functionEnhance
DynamicTableto improve support for use during acquisitionDynamicTableso it creates and initializes itsElementIdentifiersidcolumn internally duringinitialize(). This is to ensure theElementIdentifiersare always create before we start the recording. CallingDynamicTable::setRowIDs()previously required creation ofElementIdentifiersbut may be called later during recording. As part of this change,setRowIDs()now takes only the row ID values and writes them directly to the table's built-inidcolumn instead of accepting a separateElementIdentifiersobject. In practice this aligns the API with the intended usage, since callers should have always used the table's owniddataset. (@oruebel, #302)DynamicTablecolumn-name management to be immediate and table-driven.addColumnName()now flushes new column names to the file as columns are added,setColNames()is now restricted to reordering the existing set of columns (it no longer acts as a general overwrite mechanism), andfinalize()no longer writes thecolnamesattribute because column-name updates are persisted when they occur. This ensures that thecolnamesattribute is always consistent with the actual columns in the table and helps avoid creation of invalid files. (@oruebel, #304)Other
TimestampVectorDataandDurationVectorData; e.g, bad namespace and inconsistent ordering of parameters in initializeFrom #305 : Row-based Recordings
Fix #303
Problem: The current implementation of
DynamicTableis mainly designed for the purpose of metadata tables, e.g.,ElectrodeTable, where we can usually fill in the whole table before the actual acquisition. With the newEventTable, however, we need enhanced support for acquiring the data in the table as part of the acquisition workflow.Goal: Update
DynamicTableandEventTableto simplify row-based recording and configuration of the tableChanges:
addColumnData::DataSpecBaseandData::DataSpec<T>to support runtime configuration ofDynamicTablecolumn layouts and to decouple table schema definition from object initialization. EachDatasubclass (e.g.,VectorData,ElementIdentifiers,TimestampVectorData,DurationVectorData) now exposes a nestedDataSpecstruct that bundles the dataset configuration, description, and any type-specific parameters needed to create and initialize the column.DynamicTable::createDefaultDataSpecsstatic factory to return the default ordered list ofDataSpecobjects for a table (currently just theidcolumn). Subclasses (ElectrodesTable,EventsTable,MeaningsTable) override this to append their own required columns, allowing callers to retrieve, inspect, and customize the default column configuration before passing it toinitialize().DynamicTable::initialize,ElectrodesTable::initialize,EventsTable::initialize, andMeaningsTable::initializeto accept astd::vector<DataSpecPtr>column-spec list instead of individualrowChunkSize/ resolution / flag parameters.DynamicTable::addColumn(DataSpecPtr)overload consistent with the newDataSpec-based API used byinitialize()andcreateDefaultDataSpecs().DyanmicTable::validateDataSpecs(andDynamicTable::checkRequiredColumnNameshelper function) to validate column specifications as part ofDyanmicTable::initializebefore initialization. Also updated subclassed ofDynamicTable(e.g.,ElectrodesTable,EventsTable,MeaningsTable) to overridevalidateDataSpecsto provide their specific validation logic.DynamicTable::addRowandDynamicTable::addRowsmethods to support row-based data insertion into aDynamicTable. Rows are specified asDynamicTable::RowData(anunordered_mapfrom column name toCellValuevariant), enabling type-safe, column-keyed writes without requiring callers to manage per-column buffers directly. Row IDs are auto-generated if not provided.BaseDataType::BaseDataVariantscalar variant type andBaseDataType::createEmptyVectorVarianthelper to support runtime-typed single-cell and buffer operations needed byaddRow/addRows. (@cline, @oruebel, #305)MeaningsTableto a columnsschema_to_aqnwb.pyutility to follow the new structure forDynamicTable. See Update schema_to_aqnwb.py utility to follow the new structure for DynamicTable #314From #325 : Add TimeIntervals
Fix #323
Fix #324
src/nwb/epoch/TimeIntervalfor annotating time intervalssrc/nwb/hdmf/table/VectorIndexfor storing index columnsVectorData::appendDataandVectorIndex::appendDatato simplify appending values to columnsVectorData.hppto the sourceVectorData.cppfileCellValueandRowDatasto `Types to avoid circular dependencyCellValueto support both scalar and vector data for support of ragged array columnsDynamicTable::addRowsetc to support ragged array column writeDynamicTable::toStringto support converting rows to a string representationDynamicTable::getNumberOfRowsto support retrieving the number of rows in aDynamicTable.VectorIndexTimeintervalsDynamicTable::readRowsto support reading of full rows. This also movedCellValueandRowDatatoTypes.hpp. It also addedVectorData::readCellValuesandVectorIndex::readTargetCellValues.intervalstables, e.g.,trials,invalid_time,epochsTimeIntervalstype indocs/pages/userdocs/time_intervals.doxand corresponding newtests/examples/testTimeIntervalsWorkflowExamples.cpplint.ymlworkflow to installclang-formatviapipxto ensure use of the latest version ofclang-formatTypes.hppto use anamespaceinstead ofclassto group types. For more users this change should not require any code changes since the syntax for accessing the types is the same (e.g.,AQNWB::Types::Status), but for users who were usingusingdeclarations to import the type can now useusing namespace AQNWB::Types.Additional changes from #328
Fix #327.
RegisteredType::createto retrieve objects from theRecordingObjectscache of the I/O if they already exist, rather than creating a new instance. This ensures that multiple requests for the same object path return the exact same object instance.BaseRecordingDatacache fromRegisteredTypetoBaseIO, which also ensures that there is only one BaseRecordingObject per Dataset path+io. This makes the caching consistent between objects and datasets by moving all the caches to the I/O and provides stronger consistency in management of objects for users.VectorIndex::setTargetColumnfrom Add time intervals #325. This logic was problematic as it: i) exposed internal logic to avoid duplicate objects to the user and ii) requiredDynamicTableto updated caches inVectorIndex. By centralizing all the object caches this is no longer necessary.Future tasks
From #320 : Add Subject
Fix #290
nwb-inspectorupgradedcheck_subject_existsto a warning/error for DANDI compliance. AqNWB did not yet supportSubjectand was silently ignoring this check in CI via--ignore=check_subject_exists.Changes
Subjectclass to represent the/general/subjectgroup in NWB files.Subject::SubjectSpecto configure a Subject and facilitate initialization also via NWBFile.--ignore=check_subject_existsoption to require subject.NWBFile::initializeto accept aSubjectSpecargument for subject metadata initialization