From 6b1dbc6672fb13d1dc2e0e30ae3e443aac1a8f77 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel <oruebel@users.noreply.github.com> Date: Tue, 21 Jan 2025 14:48:12 -0800 Subject: [PATCH 1/5] Add codecov badge to README.md (#135) * Add codecov badge to README.md * exclude tests from coverage trace * add outputs for debugging * update coverage workflow * debug coverage dir * debug coverage dir * modify exclusion criteria * revert coverage trace command --------- Co-authored-by: Steph Prince <40640337+stephprince@users.noreply.github.com> --- README.md | 3 ++- cmake/coverage.cmake | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 249d6d96..cd231d3f 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ [](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/codespell.yml) [](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/lint.yml) [](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/doxygen-gh-pages.yml) +[](https://app.codecov.io/github/NeurodataWithoutBorders/aqnwb?branch=main) [](https://neurodatawithoutborders.github.io/aqnwb/) [](https://nwb-overview.readthedocs.io/en/latest/nwb-project-analytics/docs/source/code_stat_pages/AqNWB_stats.html) @@ -30,4 +31,4 @@ See the [AqNWB Documentation](https://neurodatawithoutborders.github.io/aqnwb/) For more information about the license, contributing guidelines, code of conduct and other relevant documentation for developers please see the -[Developer Documentation](https://neurodatawithoutborders.github.io/aqnwb/devdocs.html). \ No newline at end of file +[Developer Documentation](https://neurodatawithoutborders.github.io/aqnwb/devdocs.html). diff --git a/cmake/coverage.cmake b/cmake/coverage.cmake index 6c4b03e2..868e5dc7 100644 --- a/cmake/coverage.cmake +++ b/cmake/coverage.cmake @@ -4,7 +4,7 @@ # customization issues set( COVERAGE_TRACE_COMMAND - lcov -c -q + lcov -c --verbose -o "${PROJECT_BINARY_DIR}/coverage.info" -d "${PROJECT_BINARY_DIR}" --include "${PROJECT_SOURCE_DIR}/src/*" From d76f8b7b384ee6dcd2fb775ac9c4d0bd709f0d4b Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Tue, 21 Jan 2025 18:26:26 -0500 Subject: [PATCH 2/5] Add AnnotationSeries data type (#141) * add initial AnnotationSeries implementation * add convenience functions, remove dtype option * add draft of tests * update writeDataBlock error message * update AnnotationSeries methods for vlen strings * add tests * fix formatting * Update src/nwb/misc/AnnotationSeries.hpp Co-authored-by: Oliver Ruebel <oruebel@users.noreply.github.com> * update read example in tests --------- Co-authored-by: Oliver Ruebel <oruebel@users.noreply.github.com> --- CMakeLists.txt | 1 + src/io/hdf5/HDF5RecordingData.cpp | 3 +- src/nwb/NWBFile.cpp | 30 +++++++++++++ src/nwb/NWBFile.hpp | 14 ++++++ src/nwb/RecordingContainers.cpp | 17 +++++++ src/nwb/RecordingContainers.hpp | 16 +++++++ src/nwb/misc/AnnotationSeries.cpp | 71 ++++++++++++++++++++++++++++++ src/nwb/misc/AnnotationSeries.hpp | 71 ++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/testMisc.cpp | 73 +++++++++++++++++++++++++++++++ tests/testNWBFile.cpp | 55 +++++++++++++++++++++++ 11 files changed, 351 insertions(+), 1 deletion(-) create mode 100644 src/nwb/misc/AnnotationSeries.cpp create mode 100644 src/nwb/misc/AnnotationSeries.hpp create mode 100644 tests/testMisc.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8fbe6787..f2287f36 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,6 +32,7 @@ add_library( src/nwb/ecephys/SpikeEventSeries.cpp src/nwb/file/ElectrodeGroup.cpp src/nwb/file/ElectrodeTable.cpp + src/nwb/misc/AnnotationSeries.cpp src/nwb/hdmf/base/Container.cpp src/nwb/hdmf/base/Data.cpp src/nwb/hdmf/table/DynamicTable.cpp diff --git a/src/io/hdf5/HDF5RecordingData.cpp b/src/io/hdf5/HDF5RecordingData.cpp index c7f6d8b6..7fc80e90 100644 --- a/src/io/hdf5/HDF5RecordingData.cpp +++ b/src/io/hdf5/HDF5RecordingData.cpp @@ -53,7 +53,8 @@ Status HDF5RecordingData::writeDataBlock( || type.type == BaseDataType::Type::T_STR) { std::cerr << "HDF5RecordingData::writeDataBlock called for string data, " - "use HDF5RecordingData::writeStringDataBlock instead." + "use HDF5RecordingData::writeDataBlock with a string array " + "data input instead of void* data." << std::endl; return Status::Failure; } diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index 4d3b4743..dc23df75 100644 --- a/src/nwb/NWBFile.cpp +++ b/src/nwb/NWBFile.cpp @@ -16,6 +16,7 @@ #include "nwb/ecephys/ElectricalSeries.hpp" #include "nwb/ecephys/SpikeEventSeries.hpp" #include "nwb/file/ElectrodeGroup.hpp" +#include "nwb/misc/AnnotationSeries.hpp" #include "spec/core.hpp" #include "spec/hdmf_common.hpp" #include "spec/hdmf_experimental.hpp" @@ -298,6 +299,35 @@ Status NWBFile::createSpikeEventSeries( return Status::Success; } +Status NWBFile::createAnnotationSeries(std::vector<std::string> recordingNames, + RecordingContainers* recordingContainers, + std::vector<SizeType>& containerIndexes) +{ + if (!m_io->canModifyObjects()) { + return Status::Failure; + } + + for (size_t i = 0; i < recordingNames.size(); ++i) { + const std::string& recordingName = recordingNames[i]; + + std::string annotationSeriesPath = + AQNWB::mergePaths(acquisitionPath, recordingName); + + // Setup annotation series datasets + auto annotationSeries = + std::make_unique<AnnotationSeries>(annotationSeriesPath, m_io); + annotationSeries->initialize( + "Stores user annotations made during an experiment", + "no comments", + SizeArray {0}, + SizeArray {CHUNK_XSIZE}); + recordingContainers->addContainer(std::move(annotationSeries)); + containerIndexes.push_back(recordingContainers->size() - 1); + } + + return Status::Success; +} + template<SizeType N> void NWBFile::cacheSpecifications( const std::string& specPath, diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index a9e014ce..69ed79bf 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -131,6 +131,20 @@ class NWBFile : public Container RecordingContainers* recordingContainers = nullptr, std::vector<SizeType>& containerIndexes = emptyContainerIndexes); + /** @brief Create AnnotationSeries objects to record data into. + * Created objects are stored in recordingContainers. + * @param recordingNames vector indicating the names of the AnnotationSeries + * within the acquisition group + * @param recordingContainers The container to store the created TimeSeries. + * @param containerIndexes The indexes of the containers added to + * recordingContainers + * @return Status The status of the object creation operation. + */ + Status createAnnotationSeries( + std::vector<std::string> recordingNames, + RecordingContainers* recordingContainers = nullptr, + std::vector<SizeType>& containerIndexes = emptyContainerIndexes); + protected: /** * @brief Creates the default file structure. diff --git a/src/nwb/RecordingContainers.cpp b/src/nwb/RecordingContainers.cpp index dc21d29e..f65b785e 100644 --- a/src/nwb/RecordingContainers.cpp +++ b/src/nwb/RecordingContainers.cpp @@ -4,6 +4,7 @@ #include "nwb/ecephys/ElectricalSeries.hpp" #include "nwb/ecephys/SpikeEventSeries.hpp" #include "nwb/hdmf/base/Container.hpp" +#include "nwb/misc/AnnotationSeries.hpp" using namespace AQNWB::NWB; // Recording Container @@ -86,3 +87,19 @@ Status RecordingContainers::writeSpikeEventData(const SizeType& containerInd, return ses->writeSpike( numSamples, numChannels, data, timestamps, controlInput); } + +Status RecordingContainers::writeAnnotationSeriesData( + const SizeType& containerInd, + const SizeType& numSamples, + const std::vector<std::string> data, + const void* timestamps, + const void* controlInput) +{ + AnnotationSeries* as = + dynamic_cast<AnnotationSeries*>(getContainer(containerInd)); + + if (as == nullptr) + return Status::Failure; + + return as->writeAnnotation(numSamples, data, timestamps, controlInput); +} \ No newline at end of file diff --git a/src/nwb/RecordingContainers.hpp b/src/nwb/RecordingContainers.hpp index 33f2b1da..f57bcf0d 100644 --- a/src/nwb/RecordingContainers.hpp +++ b/src/nwb/RecordingContainers.hpp @@ -111,6 +111,22 @@ class RecordingContainers const void* timestamps, const void* controlInput = nullptr); + /** + * @brief Write AnnotationSeries data to a recordingContainer dataset. + * @param containerInd The index of the AnnotationSeries dataset within the + * AnnotationSeries containers. + * @param numSamples Number of samples in the time for the single event. + * @param data A vector of strings of data to write. + * @param timestamps A pointer to the timestamps block + * @param controlInput A pointer to the control block data (optional) + * @return The status of the write operation. + */ + Status writeAnnotationSeriesData(const SizeType& containerInd, + const SizeType& numSamples, + const std::vector<std::string> data, + const void* timestamps, + const void* controlInput = nullptr); + /** * @brief Get the number of recording containers */ diff --git a/src/nwb/misc/AnnotationSeries.cpp b/src/nwb/misc/AnnotationSeries.cpp new file mode 100644 index 00000000..5e5db340 --- /dev/null +++ b/src/nwb/misc/AnnotationSeries.cpp @@ -0,0 +1,71 @@ + +#include "nwb/misc/AnnotationSeries.hpp" + +#include "Utils.hpp" + +using namespace AQNWB::NWB; + +// AnnotationSeries +// Initialize the static registered_ member to trigger registration +REGISTER_SUBCLASS_IMPL(AnnotationSeries) + +/** Constructor */ +AnnotationSeries::AnnotationSeries(const std::string& path, + std::shared_ptr<IO::BaseIO> io) + : TimeSeries(path, io) +{ +} + +/** Destructor */ +AnnotationSeries::~AnnotationSeries() {} + +/** Initialization function*/ +void AnnotationSeries::initialize(const std::string& description, + const std::string& comments, + const SizeArray& dsetSize, + const SizeArray& chunkSize) +{ + TimeSeries::initialize( + IO::BaseDataType::V_STR, // fixed to string according to schema + "n/a", // unit fixed to "n/a" + description, + comments, + dsetSize, + chunkSize, + 1.0f, // conversion fixed to 1.0, since unit is n/a + -1.0f, // resolution fixed to -1.0 + 0.0f); // offset fixed to 0.0, since unit is n/a +} + +Status AnnotationSeries::writeAnnotation(const SizeType& numSamples, + std::vector<std::string> dataInput, + const void* timestampsInput, + const void* controlInput) +{ + std::vector<SizeType> dataShape = {numSamples}; + std::vector<SizeType> positionOffset = {this->m_samplesRecorded}; + + // Write timestamps + Status tsStatus = Status::Success; + tsStatus = this->timestamps->writeDataBlock( + dataShape, positionOffset, this->timestampsType, timestampsInput); + + // Write the data + Status dataStatus = this->data->writeDataBlock( + dataShape, positionOffset, this->m_dataType, dataInput); + + // Write the control data if it exists + if (controlInput != nullptr) { + tsStatus = this->control->writeDataBlock( + dataShape, positionOffset, this->controlType, controlInput); + } + + // track samples recorded + m_samplesRecorded += numSamples; + + if ((dataStatus != Status::Success) || (tsStatus != Status::Success)) { + return Status::Failure; + } else { + return Status::Success; + } +} \ No newline at end of file diff --git a/src/nwb/misc/AnnotationSeries.hpp b/src/nwb/misc/AnnotationSeries.hpp new file mode 100644 index 00000000..a9ac2ac1 --- /dev/null +++ b/src/nwb/misc/AnnotationSeries.hpp @@ -0,0 +1,71 @@ + +#pragma once + +#include <string> + +#include "Utils.hpp" +#include "io/BaseIO.hpp" +#include "io/ReadIO.hpp" +#include "nwb/base/TimeSeries.hpp" + +namespace AQNWB::NWB +{ +/** + * @brief TimeSeries storing text-based records about the experiment. + */ +class AnnotationSeries : public TimeSeries +{ +public: + // Register the AnnotationSeries + REGISTER_SUBCLASS(AnnotationSeries, "core") + + /** + * @brief Constructor. + * @param path The location of the AnnotationSeries in the file. + * @param io A shared pointer to the IO object. + */ + AnnotationSeries(const std::string& path, std::shared_ptr<IO::BaseIO> io); + + /** + * @brief Destructor + */ + ~AnnotationSeries(); + + /** + * @brief Initializes the AnnotationSeries + * @param description The description of the AnnotationSeries. + * @param dsetSize Initial size of the main dataset. This must be a vector + * with one element specifying the length in time. + * @param chunkSize Chunk size to use. + */ + void initialize(const std::string& description, + const std::string& comments, + const SizeArray& dsetSize, + const SizeArray& chunkSize); + + /** + * @brief Writes a channel to an AnnotationSeries dataset. + * @param numSamples The number of samples to write (length in time). + * @param dataInput A vector of strings. + * @param timestampsInput A pointer to the timestamps block. + * @param controlInput A pointer to the control block data (optional) + * @return The status of the write operation. + */ + Status writeAnnotation(const SizeType& numSamples, + const std::vector<std::string> dataInput, + const void* timestampsInput, + const void* controlInput = nullptr); + + DEFINE_FIELD(readData, + DatasetField, + std::string, + "data", + Annotations made during an experiment.) + +private: + /** + * @brief The number of samples already written per channel. + */ + SizeType m_samplesRecorded = 0; +}; +} // namespace AQNWB::NWB diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c5288219..8615085e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -18,6 +18,7 @@ add_executable(aqnwb_test testElementIdentifiers.cpp testFile.cpp testHDF5IO.cpp + testMisc.cpp testNWBFile.cpp testRecordingWorkflow.cpp testRegisteredType.cpp diff --git a/tests/testMisc.cpp b/tests/testMisc.cpp new file mode 100644 index 00000000..0770e1fc --- /dev/null +++ b/tests/testMisc.cpp @@ -0,0 +1,73 @@ +#include <H5Cpp.h> +#include <catch2/catch_test_macros.hpp> +#include <catch2/matchers/catch_matchers_all.hpp> + +#include "Types.hpp" +#include "Utils.hpp" +#include "io/BaseIO.hpp" +#include "nwb/misc/AnnotationSeries.hpp" +#include "testUtils.hpp" + +using namespace AQNWB; + +TEST_CASE("AnnotationSeries", "[misc]") +{ + // setup recording info + SizeType numSamples = 3; + std::string dataPath = "/annotations"; + std::vector<std::string> mockAnnotations = { + "Subject moved", + "Break started", + "Break ended", + }; + std::vector<double> mockTimestamps = getMockTimestamps(numSamples, 1); + std::vector<double> mockTimestamps2 = mockTimestamps; + for (double& value : mockTimestamps2) { + value += 5; + } + + SECTION("test writing annotations") + { + // setup io object + std::string path = getTestFilePath("AnnotationSeries.h5"); + std::shared_ptr<BaseIO> io = createIO("HDF5", path); + io->open(); + + // setup annotation series + NWB::AnnotationSeries as = NWB::AnnotationSeries(dataPath, io); + as.initialize( + "Test annotations", "Test comments", SizeArray {0}, SizeArray {1}); + + // write annotations multiple times to test adding to same dataset + Status writeStatus = + as.writeAnnotation(numSamples, mockAnnotations, mockTimestamps.data()); + REQUIRE(writeStatus == Status::Success); + Status writeStatus2 = + as.writeAnnotation(numSamples, mockAnnotations, mockTimestamps2.data()); + REQUIRE(writeStatus2 == Status::Success); + io->flush(); + + // Read annotations back from file + std::vector<std::string> expectedAnnotations = mockAnnotations; + expectedAnnotations.insert(expectedAnnotations.end(), + mockAnnotations.begin(), + mockAnnotations.end()); + std::vector<std::string> dataOut(expectedAnnotations.size()); + + auto readDataWrapper = as.readData(); + auto readAnnotationsDataTyped = readDataWrapper->values(); + REQUIRE(readAnnotationsDataTyped.data == expectedAnnotations); + + // Read timestamps + std::vector<double> expectedTimestamps = mockTimestamps; + expectedTimestamps.insert(expectedTimestamps.end(), + mockTimestamps2.begin(), + mockTimestamps2.end()); + std::vector<double> timestampsOut(expectedTimestamps.size()); + + auto readTimestampsWrapper = as.readTimestamps(); + auto readTimestampsDataTyped = readTimestampsWrapper->values(); + REQUIRE_THAT(readTimestampsDataTyped.data, + Catch::Matchers::Approx(expectedTimestamps)); + } +} diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index 35939c3f..9bbd8f29 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -10,6 +10,7 @@ #include "nwb/RecordingContainers.hpp" #include "nwb/base/TimeSeries.hpp" #include "nwb/ecephys/SpikeEventSeries.hpp" +#include "nwb/misc/AnnotationSeries.hpp" #include "testUtils.hpp" using namespace AQNWB; @@ -164,6 +165,60 @@ TEST_CASE("createMultipleEcephysDatasets", "[nwb]") nwbfile.finalize(); } +TEST_CASE("createAnnotationSeries", "[nwb]") +{ + std::string filename = getTestFilePath("createAnnotationSeries.nwb"); + + // initialize nwbfile object and create base structure + std::shared_ptr<IO::HDF5::HDF5IO> io = + std::make_shared<IO::HDF5::HDF5IO>(filename); + io->open(); + NWB::NWBFile nwbfile(io); + nwbfile.initialize(generateUuid()); + + // create Annotation Series + std::vector<std::string> mockAnnotationNames = {"annotations1", + "annotations2"}; + std::unique_ptr<NWB::RecordingContainers> recordingContainers = + std::make_unique<NWB::RecordingContainers>(); + Status resultCreate = nwbfile.createAnnotationSeries( + mockAnnotationNames, recordingContainers.get()); + REQUIRE(resultCreate == Status::Success); + + // start recording + Status resultStart = io->startRecording(); + REQUIRE(resultStart == Status::Success); + + // write annotation data + std::vector<std::string> mockAnnotations = { + "Start recording", "Subject moved", "End recording"}; + std::vector<double> mockTimestamps = {0.1, 0.5, 1.0}; + std::vector<SizeType> positionOffset = {0}; + SizeType dataShape = mockAnnotations.size(); + + // write to both annotation series + recordingContainers->writeAnnotationSeriesData( + 0, dataShape, mockAnnotations, mockTimestamps.data()); + recordingContainers->writeAnnotationSeriesData( + 1, dataShape, mockAnnotations, mockTimestamps.data()); + + // test searching for all AnnotationSeries objects + std::unordered_set<std::string> typesToSearch = {"core::AnnotationSeries"}; + std::unordered_map<std::string, std::string> found_types = + io->findTypes("/", typesToSearch, IO::SearchMode::CONTINUE_ON_TYPE); + REQUIRE(found_types.size() + == 2); // We should have annotations1 and annotations2 + for (const auto& pair : found_types) { + // only AnnotationSeries should be found + REQUIRE(pair.second == "core::AnnotationSeries"); + // only annotations1 or annotations2 should be in the list + REQUIRE(((pair.first == "/acquisition/annotations1") + || (pair.first == "/acquisition/annotations2"))); + } + + nwbfile.finalize(); +} + TEST_CASE("setCanModifyObjectsMode", "[nwb]") { std::string filename = getTestFilePath("testCanModifyObjectsMode.nwb"); From d71c5611b5e4b589c8099c67c9cc2f0f130dc3ff Mon Sep 17 00:00:00 2001 From: Oliver Ruebel <oruebel@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:12:31 -0800 Subject: [PATCH 3/5] Support reading of arbitrary fields (#142) * Add unit tests for BaseIO::findTypes * Add unit tests for unsupported data type * Enhance testing for RegisteredType * Update getGroupObjects to getStorageObjects * Add unit test for findType with Dataset * Fix #131 support reading of custom fields via RegisteredType * Fix codespell * Update docs/pages/userdocs/read.dox Co-authored-by: Steph Prince <40640337+stephprince@users.noreply.github.com> * Update src/Types.hpp Co-authored-by: Steph Prince <40640337+stephprince@users.noreply.github.com> * Add additional test case for HDF5IO::getStorageObjects --------- Co-authored-by: Steph Prince <40640337+stephprince@users.noreply.github.com> --- docs/pages/userdocs/read.dox | 27 +++- src/Types.hpp | 13 ++ src/io/BaseIO.cpp | 21 ++- src/io/BaseIO.hpp | 27 ++-- src/io/hdf5/HDF5IO.cpp | 53 ++++++- src/io/hdf5/HDF5IO.hpp | 23 +-- src/nwb/NWBFile.cpp | 7 +- src/nwb/RegisteredType.hpp | 45 ++++++ tests/CMakeLists.txt | 1 + tests/examples/test_ecephys_data_read.cpp | 24 ++++ tests/testBaseIO.cpp | 122 ++++++++++++++++ tests/testHDF5IO.cpp | 162 +++++++++++++++++++--- tests/testRegisteredType.cpp | 155 +++++++++++++++++++-- 13 files changed, 618 insertions(+), 62 deletions(-) create mode 100644 tests/testBaseIO.cpp diff --git a/docs/pages/userdocs/read.dox b/docs/pages/userdocs/read.dox index 363ac471..10f27958 100644 --- a/docs/pages/userdocs/read.dox +++ b/docs/pages/userdocs/read.dox @@ -308,7 +308,7 @@ * - \ref AQNWB::IO::BaseIO "BaseIO", \ref AQNWB::IO::HDF5::HDF5IO "HDF5IO" are responsible for * i) reading type attribute and group information, ii) searching the file for typed objects via * \ref AQNWB::IO::BaseIO::findTypes "findTypes()" methods, and iii) retrieving the paths of all - * object within a group via \ref AQNWB::IO::BaseIO::getGroupObjects "getGroupObjects()" + * object associated with a storage object (e.g., a Group) via \ref AQNWB::IO::BaseIO::getStorageObjects "getStorageObjects()" * * \subsubsection read_design_wrapper_registeredType RegisteredType * @@ -320,7 +320,9 @@ * methods that we can use to instantiate any registered subclass just using the ``io`` object * and ``path`` for the object in the file. \ref AQNWB::NWB::RegisteredType "RegisteredType" can read * the type information from the corresponding `namespace` and `neurodata_type` attributes to - * determine the full type and in run look up the corresponding class in its registry and create the type. + * determine the full type, then look up the corresponding class in its registry, and then create the type. + * Using \ref AQNWB::NWB::RegisteredType::readField "RegisteredType::readField" also provides a + * general mechanism for reading arbitrary fields. * * \subsubsection read_design_wrapper_subtypes Child classes of RegisteredType (e.g., Container) * @@ -540,5 +542,26 @@ * * \snippet tests/examples/test_ecephys_data_read.cpp example_read_only_stringattr_snippet * + * + * \subsubsection read_design_example_read_arbitrary_field Reading arbitrary fields + * + * Even if there is no dedicated `DEFINE_FIELD` definition available, we can still read + * any arbitrary sub-field associated with a particular \ref AQNWB::NWB::RegisteredType "RegisteredType" + * via the generic \ref AQNWB::NWB::RegisteredType::readField "RegisteredType::readField" method. The main + * difference is that for datasets and attributes we need to specify all the additional information + * (e.g., the relative path, object type, and data type) ourselves, whereas using `DEFINE_FIELD` + * this information has already been specified for us. For example, to read the data from + * the \ref AQNWB::NWB::ElectricalSeries "ElectricalSeries" we can call: + * + * \snippet tests/examples/test_ecephys_data_read.cpp example_read_generic_dataset_field_snippet + * + * Similarly, we can also read any sub-fields that are itself \ref AQNWB::NWB::RegisteredType "RegisteredType" + * objects via \ref AQNWB::NWB::RegisteredType::readField "RegisteredType::readField" (e.g., to read custom + * \ref AQNWB::NWB::VectorData "VectorData" columns of a \ref AQNWB::NWB::DynamicTable "DynamicTable"). In + * contrast to dataset and attribute fields, we here only need to specify the relative path of the field. + * \ref AQNWB::NWB::RegisteredType "RegisteredType" in turn can read the type information from the + * `neurodata_type` and `namespace` attributes in the file directly. + * + * \snippet tests/examples/test_ecephys_data_read.cpp example_read_generic_registeredtype_field_snippet */ diff --git a/src/Types.hpp b/src/Types.hpp index 03e7789f..6d45f767 100644 --- a/src/Types.hpp +++ b/src/Types.hpp @@ -35,6 +35,19 @@ class Types Undefined = -1 }; + /** + * \brief Helper struct to check if a value is a data field, i.e., + * Dataset or Attribute + * + * This function is used to enforce constraints on templated functions that + * should only be callable for valid StorageObjectType values + */ + template<StorageObjectType T> + struct IsDataStorageObjectType + : std::integral_constant<bool, (T == Dataset || T == Attribute)> + { + }; + /** * @brief Alias for the size type used in the project. */ diff --git a/src/io/BaseIO.cpp b/src/io/BaseIO.cpp index d2d7f05f..18cbe8f7 100644 --- a/src/io/BaseIO.cpp +++ b/src/io/BaseIO.cpp @@ -77,7 +77,6 @@ std::unordered_map<std::string, std::string> BaseIO::findTypes( { // Check if the current object exists as a dataset or group if (objectExists(current_path)) { - std::cout << "Current Path: " << current_path << std::endl; // Check if we have a typed object if (attributeExists(current_path + "/neurodata_type") && attributeExists(current_path + "/namespace")) @@ -92,8 +91,6 @@ std::unordered_map<std::string, std::string> BaseIO::findTypes( std::string full_type = namespace_attr.data[0] + "::" + neurodata_type_attr.data[0]; - std::cout << "Full name: " << full_type << std::endl; - // Check if the full type matches any of the given types if (types.find(full_type) != types.end()) { found_types[current_path] = full_type; @@ -103,9 +100,14 @@ std::unordered_map<std::string, std::string> BaseIO::findTypes( // object if (search_mode == SearchMode::CONTINUE_ON_TYPE) { // Get the list of objects inside the current group - std::vector<std::string> objects = getGroupObjects(current_path); + std::vector<std::pair<std::string, StorageObjectType>> objects = + getStorageObjects(current_path, StorageObjectType::Undefined); for (const auto& obj : objects) { - searchTypes(AQNWB::mergePaths(current_path, obj)); + if (obj.second == StorageObjectType::Group + || obj.second == StorageObjectType::Dataset) + { + searchTypes(AQNWB::mergePaths(current_path, obj.first)); + } } } } catch (...) { @@ -117,9 +119,14 @@ std::unordered_map<std::string, std::string> BaseIO::findTypes( else { // Get the list of objects inside the current group - std::vector<std::string> objects = getGroupObjects(current_path); + std::vector<std::pair<std::string, StorageObjectType>> objects = + getStorageObjects(current_path, StorageObjectType::Undefined); for (const auto& obj : objects) { - searchTypes(AQNWB::mergePaths(current_path, obj)); + if (obj.second == StorageObjectType::Group + || obj.second == StorageObjectType::Dataset) + { + searchTypes(AQNWB::mergePaths(current_path, obj.first)); + } } } } diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp index c6df0a89..e44d780c 100644 --- a/src/io/BaseIO.hpp +++ b/src/io/BaseIO.hpp @@ -96,12 +96,12 @@ enum class SearchMode /** * @brief Stop searching inside an object once a matching type is found. */ - STOP_ON_TYPE, + STOP_ON_TYPE = 1, /** * @brief Continue searching inside an object even after a matching type is * found. */ - CONTINUE_ON_TYPE + CONTINUE_ON_TYPE = 2, }; /** @@ -223,19 +223,26 @@ class BaseIO virtual bool attributeExists(const std::string& path) const = 0; /** - * @brief Gets the list of objects inside a group. + * @brief Gets the list of storage objects (groups, datasets, attributes) + * inside a group. * - * This function returns a vector of relative paths of all objects inside - * the specified group. If the input path is not a group (e.g., as dataset - * or attribute or invalid object), then an empty list should be - * returned. + * This function returns the relative paths and storage type of all objects + * inside the specified group. If the input path is an attribute then an empty + * list should be returned. If the input path is a dataset, then only the + * attributes will be returned. Note, this function is not recursive, i.e., + * it only looks for storage objects associated directly with the given path. * * @param path The path to the group. + * @param objectType Define which types of storage object to look for, i.e., + * only objects of this specified type will be returned. * - * @return A vector of relative paths of all objects inside the group. + * @return A vector of pairs of relative paths and their corresponding storage + * object types. */ - virtual std::vector<std::string> getGroupObjects( - const std::string& path) const = 0; + virtual std::vector<std::pair<std::string, StorageObjectType>> + getStorageObjects(const std::string& path, + const StorageObjectType& objectType = + StorageObjectType::Undefined) const = 0; /** * @brief Finds all datasets and groups of the given types in the HDF5 file. diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp index 2bbd9482..2313b05f 100644 --- a/src/io/hdf5/HDF5IO.cpp +++ b/src/io/hdf5/HDF5IO.cpp @@ -989,16 +989,61 @@ bool HDF5IO::attributeExists(const std::string& path) const return (attributePtr != nullptr); } -std::vector<std::string> HDF5IO::getGroupObjects(const std::string& path) const +std::vector<std::pair<std::string, StorageObjectType>> +HDF5IO::getStorageObjects(const std::string& path, + const StorageObjectType& objectType) const + { - std::vector<std::string> objects; - if (getH5ObjectType(path) == H5O_TYPE_GROUP) { + std::vector<std::pair<std::string, StorageObjectType>> objects; + + H5O_type_t h5Type = getH5ObjectType(path); + if (h5Type == H5O_TYPE_GROUP) { H5::Group group = m_file->openGroup(path); hsize_t num_objs = group.getNumObjs(); for (hsize_t i = 0; i < num_objs; ++i) { - objects.push_back(group.getObjnameByIdx(i)); + std::string objName = group.getObjnameByIdx(i); + H5G_obj_t objType = group.getObjTypeByIdx(i); + StorageObjectType storageObjectType; + switch (objType) { + case H5G_GROUP: + storageObjectType = StorageObjectType::Group; + break; + case H5G_DATASET: + storageObjectType = StorageObjectType::Dataset; + break; + default: + storageObjectType = StorageObjectType::Undefined; + } + if (storageObjectType == objectType + || objectType == StorageObjectType::Undefined) + { + objects.emplace_back(objName, storageObjectType); + } + } + + // Include attributes for groups + if (objectType == StorageObjectType::Attribute + || objectType == StorageObjectType::Undefined) + { + SizeType numAttrs = group.getNumAttrs(); + for (SizeType i = 0; i < numAttrs; ++i) { + H5::Attribute attr = group.openAttribute(i); + objects.emplace_back(attr.getName(), StorageObjectType::Attribute); + } + } + } else if (h5Type == H5O_TYPE_DATASET) { + if (objectType == StorageObjectType::Attribute + || objectType == StorageObjectType::Undefined) + { + H5::DataSet dataset = m_file->openDataSet(path); + SizeType numAttrs = dataset.getNumAttrs(); + for (SizeType i = 0; i < numAttrs; ++i) { + H5::Attribute attr = dataset.openAttribute(i); + objects.emplace_back(attr.getName(), StorageObjectType::Attribute); + } } } + return objects; } diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp index 45c7663a..f0f00cfd 100644 --- a/src/io/hdf5/HDF5IO.hpp +++ b/src/io/hdf5/HDF5IO.hpp @@ -294,19 +294,26 @@ class HDF5IO : public BaseIO bool attributeExists(const std::string& path) const override; /** - * @brief Gets the list of objects inside a group. + * @brief Gets the list of storage objects (groups, datasets, attributes) + * inside a group. * - * This function returns a vector of relative paths of all objects inside - * the specified group. If the input path is not a group (e.g., as dataset - * or attribute or invalid object), then an empty list should be - * returned. + * This function returns the relative paths and storage type of all objects + * inside the specified group. If the input path is an attribute then an empty + * list should be returned. If the input path is a dataset, then only the + * attributes will be returned. Note, this function is not recursive, i.e., + * it only looks for storage objects associated directly with the given path. * * @param path The path to the group. + * @param objectType Define which types of storage object to look for, i.e., + * only objects of this specified type will be returned. * - * @return A vector of relative paths of all objects inside the group. + * @return A vector of pairs of relative paths and their corresponding storage + * object types. */ - std::vector<std::string> getGroupObjects( - const std::string& path) const override; + virtual std::vector<std::pair<std::string, StorageObjectType>> + getStorageObjects(const std::string& path, + const StorageObjectType& objectType = + StorageObjectType::Undefined) const override; /** * @brief Returns the HDF5 type of object at a given path. diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index dc23df75..87d3ade4 100644 --- a/src/nwb/NWBFile.cpp +++ b/src/nwb/NWBFile.cpp @@ -66,7 +66,8 @@ Status NWBFile::initialize(const std::string& identifierText, bool NWBFile::isInitialized() const { - auto existingGroupObjects = m_io->getGroupObjects("/"); + std::vector<std::pair<std::string, StorageObjectType>> existingGroupObjects = + m_io->getStorageObjects("/", StorageObjectType::Group); if (existingGroupObjects.size() == 0) { return false; } @@ -85,8 +86,8 @@ bool NWBFile::isInitialized() const // Iterate over the existing objects and add to foundObjects if it's a // required object for (const auto& obj : existingGroupObjects) { - if (requiredObjects.find(obj) != requiredObjects.end()) { - foundObjects.insert(obj); + if (requiredObjects.find(obj.first) != requiredObjects.end()) { + foundObjects.insert(obj.first); } } diff --git a/src/nwb/RegisteredType.hpp b/src/nwb/RegisteredType.hpp index 4f71b61c..2d7b1fd0 100644 --- a/src/nwb/RegisteredType.hpp +++ b/src/nwb/RegisteredType.hpp @@ -198,6 +198,51 @@ class RegisteredType return (getNamespace() + "::" + getTypeName()); } + /** + * @brief Support reading of arbitrary fields by their relative path + * + * This function provided as a general "backup" to support reading of + * arbitrary fields even if the sub-class may not have an explicit + * DEFINE_FIELD specified for the field. If a DEFINE_FIELD exists then + * the corresponding read method should be used as it avoids the need + * for specifying most (if not all) of the function an template + * parameters needed by this function. + * + * @param fieldPath The relative path of the field within the current type, + * i.e., relative to `m_path` + * @tparam SOT The storage object type. This must be a either + * StorageObjectType::Dataset or StorageObjectType::Attribute + * @tparam VTYPE The value type of the field to be read. + * @tparam Enable SFINAE (Substitution Failure Is Not An Error) mechanism + * to enable this function only if SOT is a Dataset or Attribute. + * + * @return ReadDataWrapper object for lazy reading of the field + */ + template<StorageObjectType SOT, + typename VTYPE, + typename std::enable_if<Types::IsDataStorageObjectType<SOT>::value, + int>::type = 0> + inline std::unique_ptr<IO::ReadDataWrapper<SOT, VTYPE>> readField( + const std::string& fieldPath) const + { + return std::make_unique<IO::ReadDataWrapper<SOT, VTYPE>>( + m_io, AQNWB::mergePaths(m_path, fieldPath)); + } + + /** + * @brief Read a field that is itself a RegisteredType + * + * @param fieldPath The relative path of the field within the current type, + * i.e., relative to `m_path. The field must itself be RegisteredType + * + * @return A unique_ptr to the created instance of the subclass. + */ + inline std::shared_ptr<AQNWB::NWB::RegisteredType> readField( + const std::string& fieldPath) + { + return this->create(AQNWB::mergePaths(m_path, fieldPath), m_io); + } + protected: /** * @brief Register a subclass name and its factory function in the registry. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8615085e..77b8b9ce 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -12,6 +12,7 @@ include(Catch) # ---- Tests ---- add_executable(aqnwb_test + testBaseIO.cpp testData.cpp testDevice.cpp testEcephys.cpp diff --git a/tests/examples/test_ecephys_data_read.cpp b/tests/examples/test_ecephys_data_read.cpp index 916cc17c..ab791973 100644 --- a/tests/examples/test_ecephys_data_read.cpp +++ b/tests/examples/test_ecephys_data_read.cpp @@ -268,5 +268,29 @@ TEST_CASE("ElectricalSeriesReadExample", "[ecephys]") readElectricalSeries->readDataUnit()->values().data[0]; REQUIRE(esUnitValue == std::string("volts")); // [example_read_only_stringattr_snippet] + + // [example_read_generic_dataset_field_snippet] + // Read the data field via the generic readField method + auto readElectricalSeriesData3 = + readElectricalSeries->readField<StorageObjectType::Dataset, float>( + std::string("data")); + // Read the data values as usual + DataBlock<float> readDataValues3 = readElectricalSeriesData3->values(); + REQUIRE(readDataValues3.data.size() == (numSamples * numChannels)); + // [example_read_generic_dataset_field_snippet] + + // [example_read_generic_registeredtype_field_snippet] + // read the NWBFile + auto readNWBFile = + NWB::RegisteredType::create<AQNWB::NWB::NWBFile>("/", readio); + // read the ElectricalSeries from the NWBFile object via the readField + // method returning a generic std::shared_ptr<RegisteredType> + auto readRegisteredType = readNWBFile->readField(esdata_path); + // cast the generic pointer to the more specific ElectricalSeries + std::shared_ptr<AQNWB::NWB::ElectricalSeries> readElectricalSeries2 = + std::dynamic_pointer_cast<AQNWB::NWB::ElectricalSeries>( + readRegisteredType); + REQUIRE(readElectricalSeries2 != nullptr); + // [example_read_generic_registeredtype_field_snippet] } } diff --git a/tests/testBaseIO.cpp b/tests/testBaseIO.cpp new file mode 100644 index 00000000..01f16b67 --- /dev/null +++ b/tests/testBaseIO.cpp @@ -0,0 +1,122 @@ +#include <catch2/catch_all.hpp> + +#include "io/hdf5/HDF5IO.hpp" +#include "testUtils.hpp" + +using namespace AQNWB::IO; + +TEST_CASE("Test findTypes functionality", "[BaseIO]") +{ + std::string filename = getTestFilePath("test_findTypes.h5"); + HDF5::HDF5IO io(filename); + io.open(FileMode::Overwrite); + + SECTION("Empty file returns empty result") + { + auto result = + io.findTypes("/", {"core::NWBFile"}, SearchMode::STOP_ON_TYPE); + REQUIRE(result.empty()); + } + + SECTION("Single type at root") + { + // Create root group with type attributes + io.createGroup("/"); + io.createAttribute("core", "/", "namespace"); + io.createAttribute("NWBFile", "/", "neurodata_type"); + + auto result = + io.findTypes("/", {"core::NWBFile"}, SearchMode::STOP_ON_TYPE); + REQUIRE(result.size() == 1); + REQUIRE(result["/"] == "core::NWBFile"); + } + + SECTION("Search for dataset type") + { + // Create root group with type attributes + io.createGroup("/"); + io.createArrayDataSet( + BaseDataType::I32, SizeArray {0}, SizeArray {1}, "/dataset1"); + io.createAttribute("hdmf-common", "/dataset1", "namespace"); + io.createAttribute("VectorData", "/dataset1", "neurodata_type"); + + auto result = io.findTypes( + "/", {"hdmf-common::VectorData"}, SearchMode::STOP_ON_TYPE); + REQUIRE(result.size() == 1); + REQUIRE(result["/dataset1"] == "hdmf-common::VectorData"); + } + + SECTION("Multiple nested types with STOP_ON_TYPE") + { + // Setup hierarchy + io.createGroup("/"); + io.createAttribute("core", "/", "namespace"); + io.createAttribute("NWBFile", "/", "neurodata_type"); + + io.createGroup("/testProcessingModule"); + io.createAttribute("core", "/testProcessingModule", "namespace"); + io.createAttribute( + "ProcessingModule", "/testProcessingModule", "neurodata_type"); + + auto result = io.findTypes("/", + {"core::NWBFile", "core::ProcessingModule"}, + SearchMode::STOP_ON_TYPE); + REQUIRE(result.size() == 1); + REQUIRE(result["/"] == "core::NWBFile"); + } + + SECTION("Multiple nested types with CONTINUE_ON_TYPE") + { + // Setup hierarchy + io.createGroup("/"); + io.createAttribute("core", "/", "namespace"); + io.createAttribute("NWBFile", "/", "neurodata_type"); + + io.createGroup("/testProcessingModule"); + io.createAttribute("core", "/testProcessingModule", "namespace"); + io.createAttribute( + "ProcessingModule", "/testProcessingModule", "neurodata_type"); + + auto result = io.findTypes("/", + {"core::NWBFile", "core::ProcessingModule"}, + SearchMode::CONTINUE_ON_TYPE); + REQUIRE(result.size() == 2); + REQUIRE(result["/"] == "core::NWBFile"); + REQUIRE(result["/testProcessingModule"] == "core::ProcessingModule"); + } + + SECTION("Non-matching types are not included") + { + // Setup hierarchy + io.createGroup("/"); + io.createAttribute("core", "/", "namespace"); + io.createAttribute("NWBFile", "/", "neurodata_type"); + + io.createGroup("/testProcessingModule"); + io.createAttribute("core", "/testProcessingModule", "namespace"); + io.createAttribute( + "ProcessingModule", "/testProcessingModule", "neurodata_type"); + + auto result = + io.findTypes("/", {"core::Device"}, SearchMode::CONTINUE_ON_TYPE); + REQUIRE(result.empty()); + } + + SECTION("Missing attributes are handled gracefully") + { + // Create group with missing neurodata_type attribute + io.createGroup("/"); + io.createAttribute("core", "/", "namespace"); + + io.createGroup("/testProcessingModule"); + io.createAttribute("core", "/testProcessingModule", "namespace"); + io.createAttribute( + "ProcessingModule", "/testProcessingModule", "neurodata_type"); + + auto result = io.findTypes( + "/", {"core::ProcessingModule"}, SearchMode::CONTINUE_ON_TYPE); + REQUIRE(result.size() == 1); + REQUIRE(result["/testProcessingModule"] == "core::ProcessingModule"); + } + io.close(); +} \ No newline at end of file diff --git a/tests/testHDF5IO.cpp b/tests/testHDF5IO.cpp index f01479e3..f50831f9 100644 --- a/tests/testHDF5IO.cpp +++ b/tests/testHDF5IO.cpp @@ -128,21 +128,53 @@ TEST_CASE("createGroup", "[hdf5io]") } } -TEST_CASE("getGroupObjects", "[hdf5io]") +TEST_CASE("getStorageObjects", "[hdf5io]") { // create and open file - std::string filename = getTestFilePath("test_getGroupObjects.h5"); - IO::HDF5::HDF5IO hdf5io(filename); + std::string filename = getTestFilePath("test_getStorageObjects.h5"); + AQNWB::IO::HDF5::HDF5IO hdf5io(filename); hdf5io.open(); SECTION("empty group") { hdf5io.createGroup("/emptyGroup"); - auto groupContent = hdf5io.getGroupObjects("/emptyGroup"); + auto groupContent = hdf5io.getStorageObjects("/emptyGroup"); REQUIRE(groupContent.size() == 0); } - SECTION("group with datasets and subgroups") + SECTION("attribute") + { + int attrData1 = 42; + hdf5io.createAttribute(BaseDataType::I32, &attrData1, "/", "attr1"); + auto attributeContent = hdf5io.getStorageObjects("/attr1"); + REQUIRE(attributeContent.size() == 0); + } + + SECTION("dataset w/o attribute") + { + // Dataset without attributes + hdf5io.createArrayDataSet( + BaseDataType::I32, SizeArray {0}, SizeArray {1}, "/data"); + auto datasetContent = hdf5io.getStorageObjects("/data"); + REQUIRE(datasetContent.size() == 0); + + // Dataset with attribute + int attrData1 = 42; + hdf5io.createAttribute(BaseDataType::I32, &attrData1, "/data", "attr1"); + auto dataContent2 = hdf5io.getStorageObjects("/data"); + REQUIRE(dataContent2.size() == 1); + REQUIRE( + dataContent2[0] + == std::make_pair(std::string("attr1"), StorageObjectType::Attribute)); + } + + SECTION("invalid path") + { + auto invalidPathContent = hdf5io.getStorageObjects("/invalid/path"); + REQUIRE(invalidPathContent.size() == 0); + } + + SECTION("group with datasets, subgroups, and attributes") { hdf5io.createGroup("/data"); hdf5io.createGroup("/data/subgroup1"); @@ -152,15 +184,43 @@ TEST_CASE("getGroupObjects", "[hdf5io]") hdf5io.createArrayDataSet( BaseDataType::I32, SizeArray {0}, SizeArray {1}, "/data/dataset2"); - auto groupContent = hdf5io.getGroupObjects("/data"); - REQUIRE(groupContent.size() == 4); - REQUIRE(std::find(groupContent.begin(), groupContent.end(), "subgroup1") + // Add attributes to the group + int attrData1 = 42; + hdf5io.createAttribute(BaseDataType::I32, &attrData1, "/data", "attr1"); + int attrData2 = 43; + hdf5io.createAttribute(BaseDataType::I32, &attrData2, "/data", "attr2"); + + auto groupContent = hdf5io.getStorageObjects("/data"); + REQUIRE(groupContent.size() == 6); + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("subgroup1"), + StorageObjectType::Group)) != groupContent.end()); - REQUIRE(std::find(groupContent.begin(), groupContent.end(), "subgroup2") + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("subgroup2"), + StorageObjectType::Group)) != groupContent.end()); - REQUIRE(std::find(groupContent.begin(), groupContent.end(), "dataset1") + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("dataset1"), + StorageObjectType::Dataset)) != groupContent.end()); - REQUIRE(std::find(groupContent.begin(), groupContent.end(), "dataset2") + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("dataset2"), + StorageObjectType::Dataset)) + != groupContent.end()); + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("attr1"), + StorageObjectType::Attribute)) + != groupContent.end()); + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("attr2"), + StorageObjectType::Attribute)) != groupContent.end()); } @@ -169,17 +229,66 @@ TEST_CASE("getGroupObjects", "[hdf5io]") hdf5io.createGroup("/rootGroup1"); hdf5io.createGroup("/rootGroup2"); - auto groupContent = hdf5io.getGroupObjects("/"); + auto groupContent = hdf5io.getStorageObjects("/"); REQUIRE(groupContent.size() == 2); - REQUIRE(std::find(groupContent.begin(), groupContent.end(), "rootGroup1") + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("rootGroup1"), + StorageObjectType::Group)) + != groupContent.end()); + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("rootGroup2"), + StorageObjectType::Group)) + != groupContent.end()); + } + + SECTION("filter by object type") + { + hdf5io.createGroup("/filterGroup"); + hdf5io.createGroup("/filterGroup/subgroup1"); + hdf5io.createArrayDataSet(BaseDataType::I32, + SizeArray {0}, + SizeArray {1}, + "/filterGroup/dataset1"); + + // Add attributes to the group + int attrData = 44; + hdf5io.createAttribute( + BaseDataType::I32, &attrData, "/filterGroup", "attr1"); + + auto groupContent = + hdf5io.getStorageObjects("/filterGroup", StorageObjectType::Group); + REQUIRE(groupContent.size() == 1); + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("subgroup1"), + StorageObjectType::Group)) != groupContent.end()); - REQUIRE(std::find(groupContent.begin(), groupContent.end(), "rootGroup2") + + groupContent = + hdf5io.getStorageObjects("/filterGroup", StorageObjectType::Dataset); + REQUIRE(groupContent.size() == 1); + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("dataset1"), + StorageObjectType::Dataset)) + != groupContent.end()); + + groupContent = + hdf5io.getStorageObjects("/filterGroup", StorageObjectType::Attribute); + REQUIRE(groupContent.size() == 1); + REQUIRE(std::find(groupContent.begin(), + groupContent.end(), + std::make_pair(std::string("attr1"), + StorageObjectType::Attribute)) != groupContent.end()); } // close file hdf5io.close(); } +// END TEST_CASE("getStorageObjects", "[hdf5io]") TEST_CASE("HDF5IO; write datasets", "[hdf5io]") { @@ -1437,8 +1546,29 @@ TEST_CASE("HDF5IO; read dataset", "[hdf5io]") SECTION("read unsupported data type") { - // TODO Add a test that tries to read some unsupported data type - // such as a strange compound data type + // open file + std::string path = getTestFilePath("test_ReadUnsupportedDataType.h5"); + std::shared_ptr<IO::HDF5::HDF5IO> hdf5io = + std::make_shared<IO::HDF5::HDF5IO>(path); + hdf5io->open(); + + // Create a compound datatype + H5::CompType compoundType(sizeof(double) * 2); + compoundType.insertMember("real", 0, H5::PredType::NATIVE_DOUBLE); + compoundType.insertMember( + "imag", sizeof(double), H5::PredType::NATIVE_DOUBLE); + + // Create dataset with compound type directly using HDF5 C++ API + H5::H5File file(path, H5F_ACC_RDWR); + hsize_t dims[1] = {5}; + H5::DataSpace dataspace(1, dims); + H5::DataSet dataset = + file.createDataSet("ComplexData", compoundType, dataspace); + + // Attempt to read the dataset - should throw an exception + REQUIRE_THROWS_AS(hdf5io->readDataset("/ComplexData"), std::runtime_error); + + hdf5io->close(); } } diff --git a/tests/testRegisteredType.cpp b/tests/testRegisteredType.cpp index 5ece9a29..a5f9efcd 100644 --- a/tests/testRegisteredType.cpp +++ b/tests/testRegisteredType.cpp @@ -10,6 +10,40 @@ using namespace AQNWB::NWB; +// Test class with custom type name +class CustomNameType : public RegisteredType +{ +public: + CustomNameType(const std::string& path, std::shared_ptr<IO::BaseIO> io) + : RegisteredType(path, io) + { + } + + virtual std::string getTypeName() const override { return "CustomType"; } + virtual std::string getNamespace() const override { return "test"; } +}; + +// Test class with field definitions +class TestFieldType : public RegisteredType +{ +public: + TestFieldType(const std::string& path, std::shared_ptr<IO::BaseIO> io) + : RegisteredType(path, io) + { + } + + virtual std::string getTypeName() const override { return "TestFieldType"; } + virtual std::string getNamespace() const override { return "test"; } + + DEFINE_FIELD(testAttribute, + AttributeField, + int32_t, + "test_attr", + "Test attribute field") + DEFINE_FIELD( + testDataset, DatasetField, float, "test_dataset", "Test dataset field") +}; + TEST_CASE("RegisterType", "[base]") { SECTION("test that the registry is working") @@ -27,23 +61,14 @@ TEST_CASE("RegisterType", "[base]") auto registry = RegisteredType::getRegistry(); auto factoryMap = RegisteredType::getFactoryMap(); // TODO we are checking for at least 10 registered types because that is how - // many - // were defined at the time of implementation of this test. We know we - // will add more, but we would like to avoid having to update this test - // every time, so we are only checking for at least 10 + // many were defined at the time of implementation of this test. We know we + // will add more, but we would like to avoid having to update this test + // every time, so we are only checking for at least 10 REQUIRE(registry.size() >= 10); REQUIRE(factoryMap.size() >= 10); REQUIRE(registry.size() == factoryMap.size()); // Test that we can indeed instantiate all registered types - // This also ensures that each factory function works correctly, - // and hence, that all subtypes implement the expected constructor - // for the RegisteredType::create method. This is similar to - // checking: - // for (const auto& pair : factoryMap) { - // auto instance = pair.second(examplePath, io); - // REQUIRE(instance != nullptr); - // } std::cout << "Registered Types:" << std::endl; for (const auto& entry : factoryMap) { const std::string& subclassFullName = entry.first; @@ -73,6 +98,9 @@ TEST_CASE("RegisterType", "[base]") // Check that the examplePath is set as expected REQUIRE(instance->getPath() == examplePath); + + // Test getFullName + REQUIRE(instance->getFullName() == (typeNamespace + "::" + typeName)); } } @@ -135,5 +163,108 @@ TEST_CASE("RegisterType", "[base]") auto readTS = AQNWB::NWB::RegisteredType::create(examplePath, io); std::string readTSType = readContainer->getTypeName(); REQUIRE(readTSType == "TimeSeries"); + + // Attempt to read the TimeSeries using the generic readField method + // By providing an empty path we tell readField to read itself + std::shared_ptr<AQNWB::NWB::RegisteredType> readRegisteredType = + readContainer->readField(std::string("")); + REQUIRE(readRegisteredType != nullptr); + std::shared_ptr<AQNWB::NWB::TimeSeries> readRegisteredTypeAsTimeSeries = + std::dynamic_pointer_cast<AQNWB::NWB::TimeSeries>(readRegisteredType); + REQUIRE(readRegisteredTypeAsTimeSeries != nullptr); + } + + SECTION("test error handling for invalid type creation") + { + std::string filename = getTestFilePath("testInvalidType.h5"); + std::shared_ptr<BaseIO> io = std::make_unique<IO::HDF5::HDF5IO>(filename); + std::string examplePath("/example/path"); + + // Test creating with non-existent type name + auto invalidInstance = + RegisteredType::create("invalid::Type", examplePath, io); + REQUIRE(invalidInstance == nullptr); + + // Test creating with empty type name + auto emptyInstance = RegisteredType::create("", examplePath, io); + REQUIRE(emptyInstance == nullptr); + + // Test creating with malformed type name (missing namespace) + auto malformedInstance = + RegisteredType::create("NoNamespace", examplePath, io); + REQUIRE(malformedInstance == nullptr); + } + + SECTION("test custom type name") + { + std::string filename = getTestFilePath("testCustomType.h5"); + std::shared_ptr<BaseIO> io = std::make_unique<IO::HDF5::HDF5IO>(filename); + std::string examplePath("/example/path"); + + // Create instance of custom named type + auto customInstance = std::make_shared<CustomNameType>(examplePath, io); + REQUIRE(customInstance != nullptr); + REQUIRE(customInstance->getTypeName() == "CustomType"); + REQUIRE(customInstance->getNamespace() == "test"); + REQUIRE(customInstance->getFullName() == "test::CustomType"); + } + + SECTION("test field definitions") + { + std::string filename = getTestFilePath("testFields.h5"); + std::shared_ptr<BaseIO> io = std::make_unique<IO::HDF5::HDF5IO>(filename); + io->open(); + std::string examplePath("/test_fields"); + + // Create test instance + auto testInstance = std::make_shared<TestFieldType>(examplePath, io); + REQUIRE(testInstance != nullptr); + + // Create parent group + io->createGroup(examplePath); + + // Create test data + const int32_t attrValue = 42; + const std::vector<float> datasetValues = {1.0f, 2.0f, 3.0f}; + + // Write test data + io->createAttribute( + BaseDataType::I32, &attrValue, examplePath, "test_attr"); + auto datasetRecordingData = + io->createArrayDataSet(BaseDataType::F32, + SizeArray {3}, + SizeArray {3}, + examplePath + "/test_dataset"); + datasetRecordingData->writeDataBlock( + SizeArray {3}, SizeArray {0}, BaseDataType::F32, datasetValues.data()); + + // Test attribute field + auto attrWrapper = testInstance->testAttribute(); + REQUIRE(attrWrapper != nullptr); + auto attrData = attrWrapper->values(); + REQUIRE(attrData.data[0] == attrValue); + + // Test dataset field + auto datasetWrapper = testInstance->testDataset(); + REQUIRE(datasetWrapper != nullptr); + auto datasetData = datasetWrapper->values(); + REQUIRE(datasetData.data == datasetValues); + + // Test reading using the general readField method + // Read test_attr via readField + auto attrWrapper2 = testInstance->readField<AttributeField, int32_t>( + std::string("test_attr")); + REQUIRE(attrWrapper2 != nullptr); + auto attrData2 = attrWrapper2->values(); + REQUIRE(attrData2.data[0] == attrValue); + + // Read test_dataset via readField + auto datasetWrapper2 = testInstance->readField<DatasetField, float>( + std::string("test_dataset")); + REQUIRE(datasetWrapper2 != nullptr); + auto datasetData2 = datasetWrapper2->values(); + REQUIRE(datasetData2.data == datasetValues); + + io->close(); } } From c5ac566c57b84dc9efb1232f9d0dfb5c67eed876 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel <oruebel@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:56:27 -0800 Subject: [PATCH 4/5] Update GitHub up/download artifact version (#147) --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d0814f43..3d1cf31c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -66,7 +66,7 @@ jobs: run: ctest --output-on-failure --no-tests=error -C Release -j 2 - name: Upload artifacts - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: test-files-${{ matrix.os }} path: | @@ -127,7 +127,7 @@ jobs: steps: - name: Download test files - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: test-files-${{ matrix.os }} path: nwb_files From c305223bf17d50a715051a3e08b928baed524eb1 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel <oruebel@users.noreply.github.com> Date: Mon, 27 Jan 2025 11:47:02 -0800 Subject: [PATCH 5/5] Update src/nwb/NWBFile.hpp Co-authored-by: Steph Prince <40640337+stephprince@users.noreply.github.com> --- src/nwb/NWBFile.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index 53af1bfd..d1b29998 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -67,7 +67,6 @@ class NWBFile : public Container * @param identifierText The identifier text for the NWBFile. * @param description A description of the NWBFile session. * @param dataCollection Information about the data collection methods. - * @param */ Status initialize(const std::string& identifierText, const std::string& description = "a recording session",