Skip to content

Add meanings table#302

Open
oruebel wants to merge 20 commits into
add_event_vectordatafrom
add_meanings_table
Open

Add meanings table#302
oruebel wants to merge 20 commits into
add_event_vectordatafrom
add_meanings_table

Conversation

@oruebel

@oruebel oruebel commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Fix #284

Implement new MeaningsTable type:

  • Implement the new src/nwb/hdmf/table/MeaningsTable.* source for MeaningsTable
  • Implement comprehensive unit tests for MeaningsTable

Update DynamicTable to support adding MeaningsTables:

  • Added "DEFINE_UNNAMED_REGISTERED_FIELD(" to create readMeaningsTable for reading MeaningsTables associated with a DynamicTable. This also creates createMeaningsTableInstance methods for creatin a new MeaningsTables that is not yet initialized.
  • Added DynamicTable.createMeaningsTable methods for creating a fully initialized MeaningsTable that is associated with a particular column of the DynamicTable
  • Add unit tests to test the new DynamicTable.createMeaningsTable and DynamicTable.readMeaningsTable methods.
    - [ ] Update the docs/tutorials as necessary to describe the use of MeaningsTable alongside DynamicTable This will be part of follow-up once the full stack for EventsTable has been added

Other Changes

  • Updated ElectrodesTable.initialize to add a new optional parameter rowChunkSize. Similar to the MeaningsTable the existing ElectrodesTable also creates pre-defined columns in initialize. Currently the chunking was set in ElectrodesTable to 1, which is too small. Adding rowChunkSize increases the default to 100 and allows the caller to set the chunk size.
  • Updated the CHANGELOG

@oruebel oruebel linked an issue Jul 2, 2026 that may be closed by this pull request
8 tasks
@oruebel oruebel added the category: enhancement proposed enhancements or new features label Jul 2, 2026
@oruebel oruebel added this to the 0.4.0 milestone Jul 2, 2026
@oruebel oruebel marked this pull request as ready for review July 2, 2026 14:40
Copilot AI review requested due to automatic review settings July 2, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds first-class support for the new HDMF Common MeaningsTable type and wires it into DynamicTable, while also improving ElectrodesTable dataset chunking by making row chunk size configurable (defaulting to 100) to avoid inefficient chunking of 1.

Changes:

  • Added src/nwb/hdmf/table/MeaningsTable.* implementing the new MeaningsTable registered type and initialization logic.
  • Extended DynamicTable with support for creating/reading MeaningsTable instances (including a convenience createMeaningsTable API) and added unit tests.
  • Updated ElectrodesTable.initialize / NWBFile::createElectrodesTable and related tests to use and verify a configurable row chunk size.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/testRecordingWorkflow.cpp Updates electrodes table creation to pass a row chunk size and asserts expected HDF5 chunking.
tests/testRecordingObjects.cpp Updates electrodes table creation to pass a row chunk size and asserts expected HDF5 chunking.
tests/testNWBFile.cpp Updates electrodes table creation to pass a row chunk size and asserts expected HDF5 chunking.
tests/testMeaningsTable.cpp Adds new unit tests covering MeaningsTable initialization, chunking, linking, and data read/write.
tests/testDynamicTable.cpp Adds tests for DynamicTable::createMeaningsTable and DynamicTable::readMeaningsTable.
tests/CMakeLists.txt Adds the new testMeaningsTable.cpp to the test target.
src/nwb/NWBFile.hpp Extends createElectrodesTable API with optional rowChunkSize parameter.
src/nwb/NWBFile.cpp Passes rowChunkSize through to ElectrodesTable::initialize.
src/nwb/hdmf/table/MeaningsTable.hpp Introduces the MeaningsTable type and its registered fields (value/meaning/target).
src/nwb/hdmf/table/MeaningsTable.cpp Implements MeaningsTable::initialize (datasets + target link).
src/nwb/hdmf/table/DynamicTable.hpp Adds API/macro support for MeaningsTable plus a convenience creation method.
src/nwb/hdmf/table/DynamicTable.cpp Implements DynamicTable::createMeaningsTable (group creation + initialization).
src/nwb/file/ElectrodesTable.hpp Extends ElectrodesTable::initialize with optional rowChunkSize.
src/nwb/file/ElectrodesTable.cpp Applies rowChunkSize to dataset chunking during initialization.
CMakeLists.txt Adds MeaningsTable.cpp to the library build.

Comment thread src/nwb/hdmf/table/DynamicTable.hpp
Comment thread src/nwb/hdmf/table/DynamicTable.hpp Outdated
Comment thread src/nwb/hdmf/table/MeaningsTable.cpp
Comment thread src/nwb/hdmf/table/MeaningsTable.cpp Outdated
oruebel and others added 7 commits July 2, 2026 07:54
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Comment thread tests/CMakeLists.txt
Comment on lines 59 to 62
* @param description The description of the table (default: "metadata about
* extracellular electrodes")
* @return Status::Success if successful, otherwise Status::Failure.
*/
Comment thread src/nwb/file/ElectrodesTable.cpp
Comment thread src/nwb/hdmf/table/MeaningsTable.cpp Outdated
oruebel and others added 5 commits July 2, 2026 08:33
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@oruebel oruebel requested a review from rly July 2, 2026 21:29
@oruebel oruebel self-assigned this Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: enhancement proposed enhancements or new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for new MeaningsTable type in hdfm-common

2 participants