From 596b90821ccec51138753e0598142f7fbcadf482 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Sun, 2 Feb 2025 19:43:28 -0800 Subject: [PATCH 1/6] Support overwrite of string array attributes and setting of colnames --- src/io/BaseIO.hpp | 4 +- src/io/hdf5/HDF5IO.cpp | 10 +++- src/io/hdf5/HDF5IO.hpp | 4 +- src/nwb/hdmf/table/DynamicTable.cpp | 7 ++- tests/testHDF5IO.cpp | 86 ++++++++++++++++++++++++++--- 5 files changed, 97 insertions(+), 14 deletions(-) diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp index e44d780c..24f5af62 100644 --- a/src/io/BaseIO.hpp +++ b/src/io/BaseIO.hpp @@ -329,11 +329,13 @@ class BaseIO * @param data The string array attribute data. * @param path The location in the file to set the attribute. * @param name The name of the attribute. + * @param bool Overwrite the attribute if it already exists. * @return The status of the attribute creation operation. */ virtual Status createAttribute(const std::vector& data, const std::string& path, - const std::string& name) = 0; + const std::string& name, + const bool overwrite = false) = 0; /** * @brief Creates a string array attribute at a given location in the file. diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp index 06cb58fc..1505cf05 100644 --- a/src/io/hdf5/HDF5IO.cpp +++ b/src/io/hdf5/HDF5IO.cpp @@ -683,7 +683,8 @@ Status HDF5IO::createAttribute(const std::string& data, Status HDF5IO::createAttribute(const std::vector& data, const std::string& path, - const std::string& name) + const std::string& name, + const bool overwrite) { H5Object* loc; Group gloc; @@ -715,7 +716,12 @@ Status HDF5IO::createAttribute(const std::vector& data, try { if (loc->attrExists(name)) { - return Status::Failure; // don't allow overwriting + if (overwrite) { + // Delete the existing attribute + loc->removeAttr(name); + } else { + return Status::Failure; // don't allow overwriting + } } // Create dataspace based on number of strings diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp index 76b1952e..b6027b44 100644 --- a/src/io/hdf5/HDF5IO.hpp +++ b/src/io/hdf5/HDF5IO.hpp @@ -157,11 +157,13 @@ class HDF5IO : public BaseIO * @param data The string array attribute data. * @param path The location in the file to set the attribute. * @param name The name of the attribute. + * @param bool Overwrite the attribute if it already exists. * @return The status of the attribute creation operation. */ Status createAttribute(const std::vector& data, const std::string& path, - const std::string& name) override; + const std::string& name, + const bool overwrite = false) override; /** * @brief Creates a fixed-length string array attribute at a given location in diff --git a/src/nwb/hdmf/table/DynamicTable.cpp b/src/nwb/hdmf/table/DynamicTable.cpp index f1cfce06..8b943ce5 100644 --- a/src/nwb/hdmf/table/DynamicTable.cpp +++ b/src/nwb/hdmf/table/DynamicTable.cpp @@ -100,6 +100,11 @@ Status DynamicTable::addReferenceColumn(const std::string& name, Status DynamicTable::finalize() { - Status colNamesStatus = m_io->createAttribute(m_colNames, m_path, "colnames"); + Status colNamesStatus = m_io->createAttribute( + m_colNames, + m_path, + "colnames", + true // overwrite the attribute if it already exists + ); return colNamesStatus; } diff --git a/tests/testHDF5IO.cpp b/tests/testHDF5IO.cpp index f50831f9..33fdd278 100644 --- a/tests/testHDF5IO.cpp +++ b/tests/testHDF5IO.cpp @@ -494,33 +494,101 @@ TEST_CASE("HDF5IO; create attributes", "[hdf5io]") IO::HDF5::HDF5IO hdf5io(filename); hdf5io.open(); - hdf5io.createGroup("/data"); + const std::string groupPath = "/data"; + hdf5io.createGroup(groupPath); // single attribute SECTION("single_value") { const signed int data = 1; - hdf5io.createAttribute(BaseDataType::I32, &data, "/data", "single_value"); + const std::string attrName = "single_value"; + const std::string attrPath = mergePaths(groupPath, attrName); + hdf5io.createAttribute(BaseDataType::I32, &data, groupPath, attrName); + REQUIRE(hdf5io.attributeExists(attrPath)); + + // Read the attribute and verify the attribute data + auto readAttrGeneric = hdf5io.readAttribute(attrPath); + auto readAttrData = IO::DataBlock::fromGeneric(readAttrGeneric); + REQUIRE(readAttrData.shape.size() == 0); // Scalar attribute + REQUIRE(readAttrData.data.size() == 1); + REQUIRE(readAttrData.data[0] == data); } // integer array SECTION("int_array") { - const int data[] = {1, 2, 3, 4, 5}; - const int dataSize = sizeof(data) / sizeof(data[0]); + const std::vector data = {1, 2, 3, 4, 5}; + const std::string attrName = "int_array"; + const std::string attrPath = mergePaths(groupPath, attrName); hdf5io.createAttribute( - BaseDataType::I32, &data, "/data", "array", dataSize); - REQUIRE(hdf5io.attributeExists("/data/array")); + BaseDataType::I32, data.data(), groupPath, attrName, data.size()); + REQUIRE(hdf5io.attributeExists(attrPath)); + + // Read the attribute and verify the attribute data + auto readAttrGeneric = hdf5io.readAttribute(attrPath); + auto readAttrData = IO::DataBlock::fromGeneric(readAttrGeneric); + REQUIRE(readAttrData.shape.size() == 1); // Scalar attribute + REQUIRE(readAttrData.data.size() == 5); + REQUIRE(readAttrData.data == data); } // string array SECTION("str_array") { const std::vector data = {"col1", "col2", "col3"}; - - hdf5io.createAttribute(data, "/data", "string_array"); - REQUIRE(hdf5io.attributeExists("/data/string_array")); + const std::string attrName = "string_array"; + const std::string attrPath = mergePaths(groupPath, attrName); + + hdf5io.createAttribute(data, groupPath, attrName); + REQUIRE(hdf5io.attributeExists(attrPath)); + + // Read the attribute and verify the attribute data + auto readAttrGeneric = hdf5io.readAttribute(attrPath); + auto readAttrData = + IO::DataBlock::fromGeneric(readAttrGeneric); + REQUIRE(readAttrData.shape.size() == 1); + REQUIRE(readAttrData.data.size() == 3); + REQUIRE(readAttrData.data == data); + } + + // string array with overwrite + SECTION("str_array_overwrite") + { + const std::vector initial_data = {"initial1", "initial2"}; + const std::vector new_data = {"new1", "new2", "new3"}; + const std::string attrName = "overwrite_string_array"; + const std::string attrPath = mergePaths(groupPath, attrName); + + // Create initial attribute + REQUIRE(hdf5io.createAttribute(initial_data, groupPath, attrName) + == Status::Success); + REQUIRE(hdf5io.attributeExists(attrPath)); + + // Read the attribute and verify the attribute data + auto readAttrGeneric = hdf5io.readAttribute(attrPath); + auto readAttrData = + IO::DataBlock::fromGeneric(readAttrGeneric); + REQUIRE(readAttrData.shape.size() == 1); + REQUIRE(readAttrData.data.size() == 2); + REQUIRE(readAttrData.data == initial_data); + + // Attempt to create the same attribute without overwrite (should fail) + REQUIRE(hdf5io.createAttribute(new_data, groupPath, attrName, false) + == Status::Failure); + + // Overwrite the attribute + REQUIRE(hdf5io.createAttribute(new_data, groupPath, attrName, true) + == Status::Success); + REQUIRE(hdf5io.attributeExists(attrPath)); + + // Read the attribute and verify the attribute data + auto readAttrGenericNew = hdf5io.readAttribute(attrPath); + auto readAttrDataNew = + IO::DataBlock::fromGeneric(readAttrGenericNew); + REQUIRE(readAttrDataNew.shape.size() == 1); + REQUIRE(readAttrDataNew.data.size() == 3); + REQUIRE(readAttrDataNew.data == new_data); } // soft link From 47709f8c4687a8ac83aad56c696db00d21083f02 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Sun, 2 Feb 2025 20:04:25 -0800 Subject: [PATCH 2/6] Read colNames in DynamicTable.initialize to support appending of columns --- src/nwb/hdmf/base/Container.cpp | 9 ++++++--- src/nwb/hdmf/table/DynamicTable.cpp | 14 +++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/nwb/hdmf/base/Container.cpp b/src/nwb/hdmf/base/Container.cpp index 0fd113e5..74e8692e 100644 --- a/src/nwb/hdmf/base/Container.cpp +++ b/src/nwb/hdmf/base/Container.cpp @@ -19,9 +19,12 @@ Container::~Container() {} /** Initialize */ Status Container::initialize() { - m_io->createGroup(m_path); + auto createGroupStatus = m_io->createGroup(m_path); // setup common attributes - m_io->createCommonNWBAttributes( + auto createAttrsStatus = m_io->createCommonNWBAttributes( m_path, this->getNamespace(), this->getTypeName()); - return Status::Success; + return (createGroupStatus == Status::Success + && createAttrsStatus == Status::Success) + ? Status::Success + : Status::Failure; } diff --git a/src/nwb/hdmf/table/DynamicTable.cpp b/src/nwb/hdmf/table/DynamicTable.cpp index f23e84de..0b87698d 100644 --- a/src/nwb/hdmf/table/DynamicTable.cpp +++ b/src/nwb/hdmf/table/DynamicTable.cpp @@ -23,8 +23,20 @@ DynamicTable::~DynamicTable() {} Status DynamicTable::initialize(const std::string& description) { Status containerStatus = Container::initialize(); - if (description != "") + if (description != "") { m_io->createAttribute(description, m_path, "description"); + } + // Read the colNames attribute if it exists such that any columns + // we may add append to the existing list of columns rathern than + // replacing it. This is important for the finalize function + // to ensure that all columns are correctly listed. + const std::string colNamesPath = mergePaths(m_path, "colnames"); + if (m_io->attributeExists(colNamesPath)) { + auto colNamesGeneric = m_io->readAttribute(colNamesPath); + auto colNamesData = + IO::DataBlock::fromGeneric(colNamesGeneric); + m_colNames = colNamesData.data; + } return containerStatus; } From 8e8d6d67c1d6aff66c3704ac30deaafe80991c20 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Sun, 2 Feb 2025 20:19:58 -0800 Subject: [PATCH 3/6] Read columns in DynamicTable constructor to avoid repeat call to initialize on read --- src/nwb/hdmf/table/DynamicTable.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/nwb/hdmf/table/DynamicTable.cpp b/src/nwb/hdmf/table/DynamicTable.cpp index 0b87698d..091b20b4 100644 --- a/src/nwb/hdmf/table/DynamicTable.cpp +++ b/src/nwb/hdmf/table/DynamicTable.cpp @@ -14,6 +14,16 @@ DynamicTable::DynamicTable(const std::string& path, : Container(path, io) , m_colNames({}) { + // Read the colNames attribute if it exists such that any columns + // we may add append to the existing list of columns rathern than + // replacing it. This is important for the finalize function + // to ensure that all columns are correctly listed. + if (m_io->isOpen()) { + auto colNamesFromFile = readColNames(); + if (colNamesFromFile->exists()) { + m_colNames = colNamesFromFile->values().data; + } + } } /** Destructor */ @@ -26,17 +36,6 @@ Status DynamicTable::initialize(const std::string& description) if (description != "") { m_io->createAttribute(description, m_path, "description"); } - // Read the colNames attribute if it exists such that any columns - // we may add append to the existing list of columns rathern than - // replacing it. This is important for the finalize function - // to ensure that all columns are correctly listed. - const std::string colNamesPath = mergePaths(m_path, "colnames"); - if (m_io->attributeExists(colNamesPath)) { - auto colNamesGeneric = m_io->readAttribute(colNamesPath); - auto colNamesData = - IO::DataBlock::fromGeneric(colNamesGeneric); - m_colNames = colNamesData.data; - } return containerStatus; } From 049e296cab1ef132f602a995471df5af3bc1a73e Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Sun, 2 Feb 2025 21:13:44 -0800 Subject: [PATCH 4/6] Fix use and writing of variable length string columns in DynamicTable and ElectrodeTable --- src/nwb/file/ElectrodeTable.cpp | 8 ++++---- src/nwb/hdmf/table/DynamicTable.cpp | 18 ++++++------------ tests/CMakeLists.txt | 1 + 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/nwb/file/ElectrodeTable.cpp b/src/nwb/file/ElectrodeTable.cpp index 5de7c116..3041405e 100644 --- a/src/nwb/file/ElectrodeTable.cpp +++ b/src/nwb/file/ElectrodeTable.cpp @@ -54,14 +54,14 @@ Status ElectrodeTable::initialize(const std::string& description) AQNWB::mergePaths(m_path, "id")))); Status groupNameStatus = m_groupNamesDataset->initialize( std::unique_ptr( - m_io->createArrayDataSet(IO::BaseDataType::STR(250), + m_io->createArrayDataSet(IO::BaseDataType::V_STR, SizeArray {0}, SizeArray {1}, AQNWB::mergePaths(m_path, "group_name"))), "the name of the ElectrodeGroup this electrode is a part of"); Status locationStatus = m_locationsDataset->initialize( std::unique_ptr( - m_io->createArrayDataSet(IO::BaseDataType::STR(250), + m_io->createArrayDataSet(IO::BaseDataType::V_STR, SizeArray {0}, SizeArray {1}, AQNWB::mergePaths(m_path, "location"))), @@ -91,14 +91,14 @@ Status ElectrodeTable::finalize() { Status rowIdStatus = setRowIDs(m_electrodeDataset, m_electrodeNumbers); Status locationColStatus = addColumn(m_locationsDataset, m_locationNames); - Status grouColStatus = addReferenceColumn( + Status groupColStatus = addReferenceColumn( "group", "a reference to the ElectrodeGroup this electrode is a part of", m_groupReferences); Status groupNameColStatus = addColumn(m_groupNamesDataset, m_groupNames); Status finalizeStatus = DynamicTable::finalize(); if (rowIdStatus != Status::Success || locationColStatus != Status::Success - || grouColStatus != Status::Success + || groupColStatus != Status::Success || groupNameColStatus != Status::Success || finalizeStatus != Status::Success) { diff --git a/src/nwb/hdmf/table/DynamicTable.cpp b/src/nwb/hdmf/table/DynamicTable.cpp index 091b20b4..fa499b6f 100644 --- a/src/nwb/hdmf/table/DynamicTable.cpp +++ b/src/nwb/hdmf/table/DynamicTable.cpp @@ -48,18 +48,12 @@ Status DynamicTable::addColumn(std::unique_ptr& vectorData, return Status::Failure; } else { // write in loop because variable length string - Status writeStatus = Status::Success; - Status blockStatus = Status::Success; - for (SizeType i = 0; i < values.size(); i++) { - blockStatus = vectorData->m_dataset->writeDataBlock( - std::vector {1}, - std::vector {i}, - IO::BaseDataType::STR(values[i].size() + 1), - values); // TODO - add tests for this - if (blockStatus != Status::Success) { - writeStatus = Status::Failure; - } - } + // Write all strings in a single block + Status writeStatus = vectorData->m_dataset->writeDataBlock( + std::vector {values.size()}, + std::vector {0}, + IO::BaseDataType::V_STR, + values); m_colNames.push_back(vectorData->getName()); return writeStatus; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 77b8b9ce..9a1c7d6e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -15,6 +15,7 @@ add_executable(aqnwb_test testBaseIO.cpp testData.cpp testDevice.cpp + testDynamicTable.cpp testEcephys.cpp testElementIdentifiers.cpp testFile.cpp From e31add9413f790fd459dc70d94f5297403441f25 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Sun, 2 Feb 2025 21:14:13 -0800 Subject: [PATCH 5/6] Add basic unit tests for DynamicTable --- tests/testDynamicTable.cpp | 159 +++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 tests/testDynamicTable.cpp diff --git a/tests/testDynamicTable.cpp b/tests/testDynamicTable.cpp new file mode 100644 index 00000000..1dc12508 --- /dev/null +++ b/tests/testDynamicTable.cpp @@ -0,0 +1,159 @@ +#include +#include + +#include "io/hdf5/HDF5IO.hpp" +#include "nwb/hdmf/table/DynamicTable.hpp" +#include "testUtils.hpp" + +using namespace AQNWB; + +TEST_CASE("DynamicTable", "[table]") +{ + std::string tablePath = "/test_table"; + + SECTION("test initialization and column names") + { + std::string path = getTestFilePath("testDynamicTable.h5"); + std::shared_ptr io = createIO("HDF5", path); + io->open(); + + NWB::DynamicTable table(tablePath, io); + Status status = table.initialize("A test dynamic table"); + REQUIRE(status == Status::Success); + + // Test reading description + auto readDesc = table.readDescription()->values().data; + REQUIRE(readDesc[0] == "A test dynamic table"); + + // Test setting and reading column names + std::vector colNames = {"col1", "col2", "col3"}; + table.setColNames(colNames); + status = table.finalize(); + REQUIRE(status == Status::Success); + + auto readColNames = table.readColNames()->values().data; + REQUIRE(readColNames == colNames); + + io->close(); + } + + SECTION("test adding columns and row IDs") + { + std::string path = getTestFilePath("testDynamicTableColumns.h5"); + std::shared_ptr io = createIO("HDF5", path); + io->open(); + + NWB::DynamicTable table(tablePath, io); + Status status = table.initialize("Table with columns"); + REQUIRE(status == Status::Success); + + // Add string vector data column + std::vector values = {"value1", "value2", "value3"}; + SizeArray dataShape = {values.size()}; + SizeArray chunking = {values.size()}; + auto columnDataset = io->createArrayDataSet( + BaseDataType::V_STR, dataShape, chunking, tablePath + "/col1"); + auto vectorData = + std::make_unique(tablePath + "/col1", io); + vectorData->initialize(std::move(columnDataset), "Column 1"); + status = table.addColumn(vectorData, values); + REQUIRE(status == Status::Success); + + // Set row IDs + std::vector ids = {1, 2, 3}; + SizeArray idShape = {ids.size()}; + SizeArray idChunking = {ids.size()}; + auto idDataset = io->createArrayDataSet( + BaseDataType::I32, idShape, idChunking, tablePath + "/id"); + auto elementIDs = + std::make_unique(tablePath + "/id", io); + elementIDs->initialize(std::move(idDataset)); + status = table.setRowIDs(elementIDs, ids); + REQUIRE(status == Status::Success); + + // Finalize table + status = table.finalize(); + REQUIRE(status == Status::Success); + + io->close(); + + // Reopen file and verify data + io = createIO("HDF5", path); + io->open(); + NWB::DynamicTable readTable(tablePath, io); + + auto readColNames = readTable.readColNames()->values().data; + std::vector expectedColNames = {"col1"}; + REQUIRE(readColNames == expectedColNames); + + // Read row IDs + auto readIdsData = readTable.readId()->values().data; + REQUIRE(readIdsData == ids); + + io->close(); + } + + SECTION("test appending column to existing table") + { + // First create a table with initial columns + std::string path = getTestFilePath("testDynamicTableAppend.h5"); + { + std::shared_ptr io = createIO("HDF5", path); + io->open(); + + NWB::DynamicTable table(tablePath, io); + Status status = table.initialize("Table for appending"); + REQUIRE(status == Status::Success); + + // Add initial column + std::vector values = {"value1", "value2", "value3"}; + SizeArray dataShape = {values.size()}; + SizeArray chunking = {values.size()}; + std::string columnPath = mergePaths(tablePath, "col1"); + auto columnDataset = io->createArrayDataSet( + BaseDataType::V_STR, dataShape, chunking, columnPath); + auto vectorData = std::make_unique(columnPath, io); + vectorData->initialize(std::move(columnDataset), "Column 1"); + status = table.addColumn(vectorData, values); + REQUIRE(status == Status::Success); + + // table.setColNames({"col1"}); + status = table.finalize(); + REQUIRE(status == Status::Success); + + io->close(); + } + + // Now reopen and append new column + { + std::shared_ptr io = createIO("HDF5", path); + io->open(); + + NWB::DynamicTable table(tablePath, io); + + // Add new column + std::vector newValues = {"new1", "new2", "new3"}; + SizeArray newDataShape = {newValues.size()}; + SizeArray newChunking = {newValues.size()}; + auto newColumnDataset = io->createArrayDataSet( + BaseDataType::V_STR, newDataShape, newChunking, tablePath + "/col2"); + auto newVectorData = + std::make_unique(tablePath + "/col2", io); + newVectorData->initialize(std::move(newColumnDataset), "Column 2"); + Status status = table.addColumn(newVectorData, newValues); + REQUIRE(status == Status::Success); + + // Update column names + std::vector colNames = {"col1", "col2"}; + table.setColNames(colNames); + status = table.finalize(); + REQUIRE(status == Status::Success); + + // Verify updated column names + auto readColNames = table.readColNames()->values().data; + REQUIRE(readColNames == colNames); + + io->close(); + } + } +} From c80b9be015b749230095cdb9e5a359eee41411ed Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Sun, 2 Feb 2025 21:25:36 -0800 Subject: [PATCH 6/6] Fix tests and docs errors --- src/io/BaseIO.hpp | 2 +- src/io/hdf5/HDF5IO.hpp | 2 +- src/nwb/hdmf/table/DynamicTable.cpp | 2 +- src/nwb/hdmf/table/DynamicTable.hpp | 13 +++++++++++-- tests/testDynamicTable.cpp | 21 +++++++++++++++------ 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp index 24f5af62..1b02324c 100644 --- a/src/io/BaseIO.hpp +++ b/src/io/BaseIO.hpp @@ -329,7 +329,7 @@ class BaseIO * @param data The string array attribute data. * @param path The location in the file to set the attribute. * @param name The name of the attribute. - * @param bool Overwrite the attribute if it already exists. + * @param overwrite Overwrite the attribute if it already exists. * @return The status of the attribute creation operation. */ virtual Status createAttribute(const std::vector& data, diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp index b6027b44..cc50747f 100644 --- a/src/io/hdf5/HDF5IO.hpp +++ b/src/io/hdf5/HDF5IO.hpp @@ -157,7 +157,7 @@ class HDF5IO : public BaseIO * @param data The string array attribute data. * @param path The location in the file to set the attribute. * @param name The name of the attribute. - * @param bool Overwrite the attribute if it already exists. + * @param overwrite Overwrite the attribute if it already exists. * @return The status of the attribute creation operation. */ Status createAttribute(const std::vector& data, diff --git a/src/nwb/hdmf/table/DynamicTable.cpp b/src/nwb/hdmf/table/DynamicTable.cpp index fa499b6f..3ae80c52 100644 --- a/src/nwb/hdmf/table/DynamicTable.cpp +++ b/src/nwb/hdmf/table/DynamicTable.cpp @@ -15,7 +15,7 @@ DynamicTable::DynamicTable(const std::string& path, , m_colNames({}) { // Read the colNames attribute if it exists such that any columns - // we may add append to the existing list of columns rathern than + // we may add append to the existing list of columns rather than // replacing it. This is important for the finalize function // to ensure that all columns are correctly listed. if (m_io->isOpen()) { diff --git a/src/nwb/hdmf/table/DynamicTable.hpp b/src/nwb/hdmf/table/DynamicTable.hpp index 9843b0b8..b27ec6f8 100644 --- a/src/nwb/hdmf/table/DynamicTable.hpp +++ b/src/nwb/hdmf/table/DynamicTable.hpp @@ -41,7 +41,6 @@ class DynamicTable : public Container * column names. * * @param description The description of the table (optional). - * @param colNames Names of the columns for the table * @return Status::Success if successful, otherwise Status::Failure. */ Status initialize(const std::string& description); @@ -86,7 +85,17 @@ class DynamicTable : public Container const std::vector& values); /** - * @brief Sets the column names of the ElectrodeTable. + * @brief Sets the column names of the DynamicTable + * + * ..note:: + * For this change to take affect in the file we need to call + * finalize() after setting the column names to write the data to the file. + * + * .. warning:: + * This will overwrite any existing column names. It is up to + * the caller to ensure that all existing columns are included in the new + * list. + * * @param newColNames The vector of new column names. */ virtual void setColNames(const std::vector& newColNames) diff --git a/tests/testDynamicTable.cpp b/tests/testDynamicTable.cpp index 1dc12508..f0cac9aa 100644 --- a/tests/testDynamicTable.cpp +++ b/tests/testDynamicTable.cpp @@ -135,24 +135,33 @@ TEST_CASE("DynamicTable", "[table]") std::vector newValues = {"new1", "new2", "new3"}; SizeArray newDataShape = {newValues.size()}; SizeArray newChunking = {newValues.size()}; + std::string columnPath2 = mergePaths(tablePath, "col2"); auto newColumnDataset = io->createArrayDataSet( - BaseDataType::V_STR, newDataShape, newChunking, tablePath + "/col2"); - auto newVectorData = - std::make_unique(tablePath + "/col2", io); + BaseDataType::V_STR, newDataShape, newChunking, columnPath2); + auto newVectorData = std::make_unique(columnPath2, io); newVectorData->initialize(std::move(newColumnDataset), "Column 2"); Status status = table.addColumn(newVectorData, newValues); REQUIRE(status == Status::Success); - // Update column names - std::vector colNames = {"col1", "col2"}; - table.setColNames(colNames); + // Finalize the table status = table.finalize(); REQUIRE(status == Status::Success); // Verify updated column names + std::vector colNames = {"col1", "col2"}; auto readColNames = table.readColNames()->values().data; REQUIRE(readColNames == colNames); + // Swap the columns + colNames = {"col2", "col1"}; + table.setColNames(colNames); + status = table.finalize(); + REQUIRE(status == Status::Success); + + // Verify updated column names + auto readColNames2 = table.readColNames()->values().data; + REQUIRE(readColNames2 == colNames); + io->close(); } }