Merge branch 'set_session_time' of https://github.com/lbl-cbg/aq-nwb …

…into set_session_time

.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

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

README.md

@@ -4,6 +4,7 @@
 [![Codespell](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/codespell.yml/badge.svg)](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/codespell.yml)
 [![Lint](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/lint.yml/badge.svg)](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/lint.yml)
 [![Docs build](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/doxygen-gh-pages.yml/badge.svg)](https://github.com/NeurodataWithoutBorders/aqnwb/actions/workflows/doxygen-gh-pages.yml)
+[![Coverage](https://codecov.io/github/NeurodataWithoutBorders/aqnwb/coverage.svg?branch=main)](https://app.codecov.io/github/NeurodataWithoutBorders/aqnwb?branch=main)
 
 [![Docs](https://img.shields.io/badge/AqNWB-Docs-8A2BE2?style=flat)](https://neurodatawithoutborders.github.io/aqnwb/)
 [![Code Stats](https://img.shields.io/badge/AqNWB-Code%20Statistics-8A2BE2?style=flat)](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).
+[Developer Documentation](https://neurodatawithoutborders.github.io/aqnwb/devdocs.html).

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/*"

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
  */
 

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.
    */

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));
+          }
         }
       }
     }

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.

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;
 }
 

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.

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;
     }

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"
@@ -92,7 +93,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;
   }
@@ -111,8 +113,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);
     }
   }
 
@@ -327,6 +329,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,