Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding columns to DynamicTable results in invalid colNames #107

Open
oruebel opened this issue Sep 20, 2024 · 0 comments
Open

Adding columns to DynamicTable results in invalid colNames #107

oruebel opened this issue Sep 20, 2024 · 0 comments
Assignees
Labels
category: bug errors in the code or code behavior category: proposal proposed enhancements or new features

Comments

@oruebel
Copy link
Contributor

oruebel commented Sep 20, 2024

Currently, DynamicTable created the colNames attribute in its initalize method here:

io->createAttribute(getColNames(), path, "colnames");

Adding columns via DynamicTable.addColumn, however, does not update the colNames.

/** Add column to table */
void DynamicTable::addColumn(const std::string& name,
const std::string& colDescription,
std::unique_ptr<VectorData>& vectorData,
const std::vector<std::string>& values)
{
if (vectorData->dataset == nullptr) {
std::cerr << "VectorData dataset is not initialized" << std::endl;
} else {
// write in loop because variable length string
for (SizeType i = 0; i < values.size(); i++)
vectorData->dataset->writeDataBlock(
std::vector<SizeType>(1, 1),
BaseDataType::STR(values[i].size() + 1),
values[i].c_str()); // TODO - add tests for this
io->createCommonNWBAttributes(
path + name, "hdmf-common", "VectorData", colDescription);
}
}

As such, we must set colNames before calling DynamicTable as otherwise the colNames will not include any added columns, and is hence invalid.

ElectrodeTable works around this issue, by: 1) fixingcolNames on construction, and 2) adding a finalize method in which all columns are added at once, rather than allowing columns to be added individually via addColumn

void ElectrodeTable::finalize()
{
setRowIDs(electrodeDataset, electrodeNumbers);
addColumn("group_name",
"the name of the ElectrodeGroup this electrode is a part of",
groupNamesDataset,
groupNames);
addColumn("location",
"the location of channel within the subject e.g. brain region",
locationsDataset,
locationNames);
addColumn("group",
"a reference to the ElectrodeGroup this electrode is a part of",
groupReferences);
}

However, if a user would call addColumn on ``ElectrodeTableto add other columns it would result in a badcolNames`.

Possible solutions

  • 1) Add a DynamicTable.finalize method and set colNames at the end rather than in initialize so that we can update colNames when adding new columns
    • The disadvantage of this is that the DynamicTable is not valid until finalize is called. This is not too bad for metadata tables that are being created and finalized at the beginning, however, this is problematic for tables that we may want to acquire data into, e.g,. TimeIntervals or PlaneSegmentation.
  • 2) Update colNames dynamically, i.e., overwrite the attribute each time a new column is being added
    • The disadvantage of this is that we potentially update the attribute repeatedly but provides maximum flexibility
      3) Add a DynanmicTable.finalizeColumns method to finish defining all columns to only allow adding new rows but not new columns.
    • This seems like a possible compromise and would be consistent with the behavior of SWMR mode in the HDFIO backend. I.e., we would call this function before enabling SWMR mode. We could potentially generalize this, and have a Container.finalizeFields function so that we could all finalizeFields for all Containers in the NWBFile

We could finalizeColumns

@oruebel oruebel added category: bug errors in the code or code behavior category: proposal proposed enhancements or new features labels Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: bug errors in the code or code behavior category: proposal proposed enhancements or new features
Projects
None yet
Development

No branches or pull requests

2 participants