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

Update handling of colnames #149

Merged
merged 27 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a38eb84
Move DynamicTable::colNames from constructor to initalize
oruebel Jan 30, 2025
ff42a15
Remove unused methods from ElectrodeTable
oruebel Jan 30, 2025
b64c257
Support adding of columns to DynamicTable
oruebel Jan 31, 2025
ae82cce
Fix #150 allow writing of variable-length string array attributes
oruebel Jan 31, 2025
0d02c02
Merge branch 'main' into move_colnames
oruebel Jan 31, 2025
394c693
Fix Windows build issue
oruebel Jan 31, 2025
df9267d
Remove colNames from DynamicTable.initialize
oruebel Jan 31, 2025
d35cd56
Return status for DynamicTable write functions
oruebel Jan 31, 2025
596b908
Support overwrite of string array attributes and setting of colnames
oruebel Feb 3, 2025
b580a23
Fix uninitalized variable warning
oruebel Feb 3, 2025
ca4db11
Merge branch 'move_colnames' into overwrite_colnames
oruebel Feb 3, 2025
47709f8
Read colNames in DynamicTable.initialize to support appending of columns
oruebel Feb 3, 2025
8e8d6d6
Read columns in DynamicTable constructor to avoid repeat call to init…
oruebel Feb 3, 2025
049e296
Fix use and writing of variable length string columns in DynamicTable…
oruebel Feb 3, 2025
e31add9
Add basic unit tests for DynamicTable
oruebel Feb 3, 2025
c80b9be
Fix tests and docs errors
oruebel Feb 3, 2025
a1057b4
Merge pull request #151 from NeurodataWithoutBorders/overwrite_colnames
oruebel Feb 3, 2025
b3d040c
Clarify detail in read.dox
oruebel Feb 3, 2025
03b8e71
Define && and || operator for the Status enum to simplify code
oruebel Feb 3, 2025
d69d55c
Fix linter error
oruebel Feb 5, 2025
d81b768
Merge branch 'main' into move_colnames
oruebel Feb 11, 2025
8cb826f
Update src/nwb/file/ElectrodeTable.hpp
oruebel Feb 11, 2025
9217da3
Remove fixed length string attribute function
oruebel Feb 11, 2025
c24ae7b
Update createAttribute for scalar strings to actually write scalar st…
oruebel Feb 11, 2025
bad4ce7
Merge branch 'main' into move_colnames
oruebel Feb 13, 2025
dbfde1f
Update fixes after merge
oruebel Feb 13, 2025
044e2d3
Fix creation of string array attribute with a single value via HDF5IO…
oruebel Feb 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions docs/pages/userdocs/read.dox
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,12 @@
* - \ref AQNWB::IO::BaseIO "BaseIO", \ref AQNWB::IO::HDF5::HDF5IO "HDF5IO" responsible for
* reading data from disk and allocating memory for data on read
* - \ref AQNWB::IO::DataBlockGeneric "DataBlockGeneric" represents a generic, n-dimensional block of data
* loaded from a file, storing the data as a generic ``std::any`` along with the ``shape`` of the data.
* - \ref AQNWB::IO::DataBlock "DataBlock" represents a typed, n-dimensional block of data, derived
* from a \ref AQNWB::IO::DataBlockGeneric "DataBlockGeneric"
* loaded from a file, storing the data as a generic ``std::any`` along with the ``shape`` of the data
* (i.e., the ``std::any`` represents a ``std::vector<DTYPE>`` so that we can cast it to the
* ``std::any_cast<std::vector<DTYPE>>(genericDataBlock.data)`` without having to copy data)
* - \ref AQNWB::IO::DataBlock "DataBlock" represents a typed, n-dimensional block of data, derived
* from a \ref AQNWB::IO::DataBlockGeneric "DataBlockGeneric". I.e., here the data has been
* cast to the correct ``std::vector<DTYPE>``.
* - \ref AQNWB::IO::ReadDataWrapper "ReadDataWrapper", is a simple wrapper class that represents
* a dataset/attribute for read, enabling lazy data read and allowing for transparent use of different I/O backends.
* - \ref AQNWB::NWB::Container "Container" type classes represent Groups with an assigned ``neurodata_type``
Expand Down
22 changes: 22 additions & 0 deletions src/Types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,28 @@ class Types
Failure = -1
};

/**
* @brief Overloaded && operator for Status enum
* @param lhs Left-hand side Status
* @param rhs Right-hand side Status
* @return Success if both statuses are Success, Failure otherwise
*/
friend Status operator&&(Status lhs, Status rhs)
{
return (lhs == Success && rhs == Success) ? Success : Failure;
}

/**
* @brief Overloaded || operator for Status enum
* @param lhs Left-hand side Status
* @param rhs Right-hand side Status
* @return Success if either status is Success, Failure otherwise
*/
friend Status operator||(Status lhs, Status rhs)
{
return (lhs == Success || rhs == Success) ? Success : Failure;
}

/**
* @brief Types of object used in the NWB schema
*/
Expand Down
23 changes: 8 additions & 15 deletions src/io/BaseIO.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,35 +318,28 @@ class BaseIO
* @param data The string attribute data.
* @param path The location in the file to set the attribute.
* @param name The name of the attribute.
* @param overwrite Overwrite the attribute if it already exists.
* @return The status of the attribute creation operation.
*/
virtual Status createAttribute(const std::string& 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.
* @brief Creates an array of variable length strings attribute at a given
* location in the file.
*
* @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 overwrite Overwrite the attribute if it already exists.
* @return The status of the attribute creation operation.
*/
virtual Status createAttribute(const std::vector<std::string>& data,
const std::string& path,
const std::string& name) = 0;

/**
* @brief Creates a string array attribute at a given location in the file.
* @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 maxSize The maximum size of the string.
* @return The status of the attribute creation operation.
*/
virtual Status createAttribute(const std::vector<const char*>& data,
const std::string& path,
const std::string& name,
const SizeType& maxSize) = 0;
const bool overwrite = false) = 0;

/**
* @brief Sets an object reference attribute for a given location in the file.
Expand Down
125 changes: 92 additions & 33 deletions src/io/hdf5/HDF5IO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Status HDF5IO::open()

Status HDF5IO::open(FileMode mode)
{
int accFlags = 0;
unsigned int accFlags = 0;

if (m_opened) {
return Status::Failure;
Expand Down Expand Up @@ -673,33 +673,76 @@ Status HDF5IO::createAttribute(const IO::BaseDataType& type,

Status HDF5IO::createAttribute(const std::string& data,
const std::string& path,
const std::string& name)
const std::string& name,
const bool overwrite)
stephprince marked this conversation as resolved.
Show resolved Hide resolved
{
std::vector<const char*> dataPtrs;
dataPtrs.push_back(data.c_str());
H5Object* loc;
Group gloc;
DataSet dloc;
Attribute attr;

return createAttribute(dataPtrs, path, name, data.length());
}
if (!canModifyObjects()) {
return Status::Failure;
}

Status HDF5IO::createAttribute(const std::vector<std::string>& data,
const std::string& path,
const std::string& name)
{
std::vector<const char*> dataPtrs;
SizeType maxLength = 0;
for (const std::string& str : data) {
SizeType length = str.length();
maxLength = std::max(maxLength, length);
dataPtrs.push_back(str.c_str());
// Create variable length string type
StrType H5type(PredType::C_S1, H5T_VARIABLE);

// Open the group or dataset
H5O_type_t objectType = getH5ObjectType(path);
switch (objectType) {
case H5O_TYPE_GROUP:
gloc = m_file->openGroup(path);
loc = &gloc;
break;
case H5O_TYPE_DATASET:
dloc = m_file->openDataSet(path);
loc = &dloc;
break;
default:
return Status::Failure; // Not a valid dataset or group type
}

return createAttribute(dataPtrs, path, name, maxLength + 1);
try {
if (loc->attrExists(name)) {
if (overwrite) {
loc->removeAttr(name);
} else {
return Status::Failure; // Don't allow overwriting
}
}

// Create dataspace for scalar
DataSpace attr_dataspace(H5S_SCALAR);

// Create the attribute
attr = loc->createAttribute(name, H5type, attr_dataspace);

// Write the scalar string data
const char* dataPtr = data.c_str();
attr.write(H5type, &dataPtr);

} catch (GroupIException& error) {
error.printErrorStack();
return Status::Failure;
} catch (AttributeIException& error) {
error.printErrorStack();
return Status::Failure;
} catch (FileIException& error) {
error.printErrorStack();
return Status::Failure;
} catch (DataSetIException& error) {
error.printErrorStack();
return Status::Failure;
}

return Status::Success;
}

Status HDF5IO::createAttribute(const std::vector<const char*>& data,
Status HDF5IO::createAttribute(const std::vector<std::string>& data,
const std::string& path,
const std::string& name,
const SizeType& maxSize)
const bool overwrite)
{
H5Object* loc;
Group gloc;
Expand All @@ -711,8 +754,8 @@ Status HDF5IO::createAttribute(const std::vector<const char*>& data,
return Status::Failure;
}

StrType H5type(PredType::C_S1, maxSize);
H5type.setSize(H5T_VARIABLE);
// Create variable length string type
StrType H5type(PredType::C_S1, H5T_VARIABLE);

// open the group or dataset
H5O_type_t objectType = getH5ObjectType(path);
Expand All @@ -731,28 +774,44 @@ Status HDF5IO::createAttribute(const std::vector<const char*>& data,

try {
if (loc->attrExists(name)) {
return Status::Failure; // don't allow overwriting because string
// attributes cannot change size easily
} else {
DataSpace attr_dataspace;
SizeType nStrings = data.size();
if (nStrings > 1) {
dims[0] = nStrings;
attr_dataspace = DataSpace(1, dims);
} else
attr_dataspace = DataSpace(H5S_SCALAR);
attr = loc->createAttribute(name, H5type, attr_dataspace);
if (overwrite) {
// Delete the existing attribute
loc->removeAttr(name);
} else {
return Status::Failure; // don't allow overwriting
}
}
attr.write(H5type, data.data());

// Create dataspace based on number of strings
DataSpace attr_dataspace;
dims[0] = data.size();
attr_dataspace = DataSpace(1, dims);

// Create the attribute
attr = loc->createAttribute(name, H5type, attr_dataspace);

// Write the data directly from the vector of strings
std::vector<const char*> dataPtrs;
dataPtrs.reserve(data.size());
for (const auto& str : data) {
dataPtrs.push_back(str.c_str());
}
attr.write(H5type, dataPtrs.data());

} catch (GroupIException error) {
error.printErrorStack();
return Status::Failure;
} catch (AttributeIException error) {
error.printErrorStack();
return Status::Failure;
} catch (FileIException error) {
error.printErrorStack();
return Status::Failure;
} catch (DataSetIException error) {
error.printErrorStack();
return Status::Failure;
}

return Status::Success;
}

Expand Down
23 changes: 8 additions & 15 deletions src/io/hdf5/HDF5IO.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,35 +145,28 @@ class HDF5IO : public BaseIO
* @param data The string attribute data.
* @param path The location in the file to set the attribute.
* @param name The name of the attribute.
* @param overwrite Overwrite the attribute if it already exists.
* @return The status of the attribute creation operation.
*/
Status createAttribute(const std::string& data,
const std::string& path,
const std::string& name) override;
const std::string& name,
const bool overwrite = false) override;

/**
* @brief Creates a string array attribute at a given location in the file.
* @brief Creates an array of variable length strings attribute at a given
* location in the file.
*
* @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 overwrite Overwrite the attribute if it already exists.
* @return The status of the attribute creation operation.
*/
Status createAttribute(const std::vector<std::string>& data,
const std::string& path,
const std::string& name) override;

/**
* @brief Creates a string array attribute at a given location in the file.
* @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 maxSize The maximum size of the string.
* @return The status of the attribute creation operation.
*/
Status createAttribute(const std::vector<const char*>& data,
const std::string& path,
const std::string& name,
const SizeType& maxSize) override;
const bool overwrite = false) override;

/**
* @brief Sets an object reference attribute for a given location in the file.
Expand Down
17 changes: 15 additions & 2 deletions src/nwb/RegisteredType.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <filesystem>
#include <functional>
#include <memory>
#include <string>
Expand Down Expand Up @@ -68,6 +69,18 @@ class RegisteredType
*/
inline std::string getPath() const { return m_path; }

/**
* @brief Get the name of the object
*
* This is the last part of getPath(), much like the filename portion
* of a file system path.
* @return String with the name of the object
*/
inline std::string getName() const
{
return std::filesystem::path(m_path).filename().string();
}

/**
* @brief Get a shared pointer to the IO object.
* @return Shared pointer to the IO object.
Expand Down Expand Up @@ -186,14 +199,14 @@ class RegisteredType
virtual std::string getNamespace() const;

/**
* @brief Get the full name of type, i.e., `namespace::typename`
* @brief Get the full name of the type, i.e., `namespace::typename`
*
* This is just a simple convenience function that uses the getNamespace
* and getTypeName methods.
*
* @return The full name of the type consisting of `namespace::typename`
*/
inline std::string getFullName() const
inline std::string getFullTypeName() const
{
return (getNamespace() + "::" + getTypeName());
}
Expand Down
6 changes: 1 addition & 5 deletions src/nwb/base/TimeSeries.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,5 @@ Status TimeSeries::writeData(const std::vector<SizeType>& dataShape,
controlShape, controlPositionOffset, this->controlType, controlInput);
}

if ((dataStatus != Status::Success) || (tsStatus != Status::Success)) {
return Status::Failure;
} else {
return Status::Success;
}
return dataStatus && tsStatus;
}
Loading
Loading