Add support for reading reference attributes (#158)

* change electrodeInd vector data type * define fields for electrodes * add tests for electrodes dataset in electricalseries data write * Support reading of reference attributes * Support reading of attribute references to registered types * Support reading of the electrodes/table attribute in ElectricalSeries * Use std::vector<char> instead of char array in readReferenceAttribute * Minor doc fixes in ElectricalSeries * Minor doc fixes in ElectricalSeries * Revert renaming of readElectrodes * Revert renaming of readElectrodes * Remove unused variable from unit test * Update docs/pages/devdocs/registered_types.dox Co-authored-by: Steph Prince <[email protected]> * Update src/nwb/RegisteredType.hpp Co-authored-by: Steph Prince <[email protected]> * Update src/nwb/ecephys/ElectricalSeries.hpp Co-authored-by: Steph Prince <[email protected]> * Fix linter error * Add unit test to read a non-reference attribute as a reference attribute * Added comment and exclude from codecov for extra runtime error checks * Add TODO note for deprecated function --------- Co-authored-by: Steph Prince <[email protected]>
NeurodataWithoutBorders · Feb 22, 2025 · ae989a3 · ae989a3
1 parent 40fe1dc
commit ae989a3
Show file tree

Hide file tree

Showing 9 changed files with 211 additions and 12 deletions.
diff --git a/docs/Doxyfile.in b/docs/Doxyfile.in
@@ -17,6 +17,7 @@ PROJECT_NUMBER = "@PROJECT_VERSION@"
 MACRO_EXPANSION = YES
 PREDEFINED           += "DEFINE_FIELD(name, storageObjectType, default_type, fieldPath, description)=/** description */ template<typename VTYPE = default_type> inline std::unique_ptr<IO::ReadDataWrapper<storageObjectType, VTYPE>> name() const;"
 PREDEFINED           += "DEFINE_REGISTERED_FIELD(name, registeredType, fieldPath, description)=/** description */ template<typename RTYPE = registeredType> inline std::shared_ptr<RTYPE> name() const;" 
+PREDEFINED           += "DEFINE_REFERENCED_REGISTERED_FIELD(name, registeredType, fieldPath, description)=/** description */ template<typename RTYPE = registeredType> inline std::shared_ptr<RTYPE> name() const;" 
 EXPAND_ONLY_PREDEF   = YES
 
 

diff --git a/docs/pages/devdocs/registered_types.dox b/docs/pages/devdocs/registered_types.dox
@@ -213,7 +213,7 @@
  *
  * @section use_the_define_registered_field_macro How to use the DEFINE_REGISTERED_FIELD macro
  *
- * The \ref DEFINE_REGISTERED_FIELD works much like the \ref DEFINE_FIELD macro macro but 
+ * The \ref DEFINE_REGISTERED_FIELD macro works much like the \ref DEFINE_FIELD macro macro but 
  * returns instances of specific subtypes of \ref AQNWB::NWB::RegisteredType "RegisteredType", 
  * rather than \ref AQNWB::IO::ReadDataWrapper "ReadDataWrapper". As such the main inputs for 
  * \ref DEFINE_REGISTERED_FIELD are as follows:
@@ -230,6 +230,16 @@
  * DEFINE_REGISTERED_FIELD(getData, DynamicTable, "my_table", My data table)
  * @endcode
  *
+ * @section use_the_define_referenced_registered_field_macro How to use the DEFINE_REFERENCED_REGISTERED_FIELD macro
+ *
+ * The \ref DEFINE_REFERENCED_REGISTERED_FIELD macro works exactly like the 
+ * \ref DEFINE_REGISTERED_FIELD macro, but the underlying data is an attribute that 
+ * stores a reference to an instances of a specific subtype of \ref AQNWB::NWB::RegisteredType "RegisteredType"
+ * rather than the instance of the object directly. I.e., ``fieldPath`` here is the
+ * relative path to the attribute that stores the reference, rather than the relative path
+ * of the object itself. The generated read method then resolves the reference first and
+ * then returns the instance of the object that is being referenced.
+ *
  * @section using_registered_subclass_with_typename Using REGISTER_SUBCLASS_WITH_TYPENAME
  *
  * The main use case for \ref REGISTER_SUBCLASS_WITH_TYPENAME is when we need to implement

diff --git a/src/io/BaseIO.hpp b/src/io/BaseIO.hpp
@@ -298,6 +298,15 @@ class BaseIO
    */
   virtual DataBlockGeneric readAttribute(const std::string& dataPath) const = 0;
 
+  /**
+   * @brief Reads a reference attribute and returns the path to the referenced
+   * object.
+   * @param dataPath The path to the reference attribute within the file.
+   * @return The path to the referenced object.
+   */
+  virtual std::string readReferenceAttribute(
+      const std::string& dataPath) const = 0;
+
   /**
    * @brief Creates an attribute at a given location in the file.
    * @param type The base data type of the attribute.

diff --git a/src/io/hdf5/HDF5IO.cpp b/src/io/hdf5/HDF5IO.cpp
@@ -301,6 +301,69 @@ std::vector<std::string> HDF5IO::readStringDataHelper(
       dataSource, numElements, H5::DataSpace(), H5::DataSpace());
 }
 
+std::string HDF5IO::readReferenceAttribute(const std::string& dataPath) const
+{
+  // Read the attribute
+  auto attributePtr = this->getAttribute(dataPath);
+  if (attributePtr == nullptr) {
+    throw std::invalid_argument(
+        "HDF5IO::readReferenceAttribute, attribute does not exist.");
+  }
+
+  H5::Attribute& attribute = *attributePtr;
+  H5::DataType dataType = attribute.getDataType();
+
+  // Check if the attribute is a reference
+  if (dataType != H5::PredType::STD_REF_OBJ) {
+    throw std::invalid_argument(
+        "HDF5IO::readReferenceAttribute, attribute is not a reference.");
+  }
+
+  // Read the reference
+  hobj_ref_t ref;
+  attribute.read(dataType, &ref);
+
+  // Dereference the reference to get the HDF5 object ID
+  // TODO: Note as of HDF5-1.12, H5Rdereference2() has been deprecated in
+  //       favor of H5Ropen_attr(), H5Ropen_object() and H5Ropen_region().
+  hid_t obj_id =
+      H5Rdereference2(attribute.getId(), H5P_DEFAULT, H5R_OBJECT, &ref);
+  if (obj_id < 0) {
+    throw std::runtime_error(
+        "HDF5IO::readReferenceAttribute, failed to dereference object.");
+  }
+
+  // Get the name (path) of the dereferenced object
+  ssize_t buf_size = H5Iget_name(obj_id, nullptr, 0) + 1;
+  // LCOV_EXCL_START
+  // This is a safety check to safeguard against possible runtime issues,
+  // but this should never happen.
+  if (buf_size <= 0) {
+    H5Oclose(obj_id);
+    throw std::runtime_error(
+        "HDF5IO::readReferenceAttribute, failed to get object name size.");
+  }
+  // LCOV_EXCL_STOP
+
+  std::vector<char> obj_name(static_cast<size_t>(buf_size));
+  // LCOV_EXCL_START
+  // This is a safety check to safeguard against possible runtime issues,
+  // but this should never happen.
+  if (H5Iget_name(obj_id, obj_name.data(), static_cast<size_t>(buf_size)) < 0) {
+    H5Oclose(obj_id);
+    throw std::runtime_error(
+        "HDF5IO::readReferenceAttribute, failed to get object name.");
+  }
+  // LCOV_EXCL_STOP
+
+  std::string referencedPath(obj_name.data());
+
+  // Close the dereferenced object
+  H5Oclose(obj_id);
+
+  return referencedPath;
+}
+
 AQNWB::IO::DataBlockGeneric HDF5IO::readAttribute(
     const std::string& dataPath) const
 {

diff --git a/src/io/hdf5/HDF5IO.hpp b/src/io/hdf5/HDF5IO.hpp
@@ -125,6 +125,15 @@ class HDF5IO : public BaseIO
   AQNWB::IO::DataBlockGeneric readAttribute(
       const std::string& dataPath) const override;
 
+  /**
+   * @brief Reads a reference attribute and returns the path to the referenced
+   * object.
+   * @param dataPath The path to the reference attribute within the file.
+   * @return The path to the referenced object.
+   */
+  std::string readReferenceAttribute(
+      const std::string& dataPath) const override;
+
   /**
    * @brief Creates an attribute at a given location in the file.
    * @param type The base data type of the attribute.

diff --git a/src/nwb/RegisteredType.hpp b/src/nwb/RegisteredType.hpp
@@ -427,5 +427,55 @@ class RegisteredType
     return nullptr; \
   }
 
+/**
+ * @brief Defines a lazy-loaded accessor function for reading fields that are
+ * RegisteredTypes that are linked to by a reference attribute
+ *
+ * This macro generates a function that returns the appropriate subtype of
+ * RegisteredType, e.g., to read VectorData from a DynamicTable or a
+ * TimeSeries from an NWBFile.
+ *
+ * \note
+ * The Doxyfile.in defines a simplified expansion of this function
+ * for generating the documentation for the autogenerated function.
+ * This means: 1) When updating the macro here, we also need to ensure
+ * that the expansion in the Doxyfile.in is still accurate and 2) the
+ * docstring that is defined by the macro here is not being used by
+ * Doxygen but the version generated by its on PREDEFINED expansion.
+ *
+ * @param name The name of the function to generate.
+ * @param registeredType The specific subclass of registered type to use
+ * @param fieldPath The path to the attribute that stores reference to the field
+ * @param description A detailed description of the field.
+ */
+#define DEFINE_REFERENCED_REGISTERED_FIELD( \
+    name, registeredType, fieldPath, description) \
+  /** \
+   * @brief Returns the instance of the class representing the ##name field. \
+   * \
+   * @tparam RTYPE The RegisteredType of the field (default: ##registeredType) \
+   * In most cases this should not be changed. But in the case of templated \
+   * types, e.g,. VectorData<std::any> a user may want to change this to a \
+   * more specific subtype to use, e.g., VectorData<int> \
+   * @return A shared pointer to an instance of ##registeredType representing \
+   * the object. May return nullptr if the path does not exist \
+   * \
+   * description \
+   */ \
+  template<typename RTYPE = registeredType> \
+  inline std::shared_ptr<RTYPE> name() const \
+  { \
+    try { \
+      std::string attrPath = AQNWB::mergePaths(m_path, fieldPath); \
+      std::string objectPath = m_io->readReferenceAttribute(attrPath); \
+      if (m_io->objectExists(objectPath)) { \
+        return RegisteredType::create<RTYPE>(objectPath, m_io); \
+      } \
+    } catch (const std::exception& e) { \
+      return nullptr; \
+    } \
+    return nullptr; \
+  }
+
 }  // namespace NWB
 }  // namespace AQNWB
diff --git a/src/nwb/ecephys/ElectricalSeries.hpp b/src/nwb/ecephys/ElectricalSeries.hpp
@@ -7,6 +7,7 @@
 #include "io/BaseIO.hpp"
 #include "io/ReadIO.hpp"
 #include "nwb/base/TimeSeries.hpp"
+#include "nwb/file/ElectrodeTable.hpp"
 
 namespace AQNWB::NWB
 {
@@ -109,17 +110,26 @@ class ElectricalSeries : public TimeSeries
                Base unit of measurement for working with the data. 
                This value is fixed to volts)
 
-  DEFINE_FIELD(readElectrodes,
-               DatasetField,
-               int,
-               "electrodes",
-               The electrodes that generated this electrical series.)
+  DEFINE_FIELD(
+      readElectrodes,
+      DatasetField,
+      int,
+      "electrodes",
+      The indices of the electrodes that generated this electrical series.)
 
   DEFINE_FIELD(readElectrodesDescription,
                AttributeField,
                std::string,
                "electrodes/description",
                The electrodes that generated this electrical series.)
+
+  DEFINE_REFERENCED_REGISTERED_FIELD(
+      readElectrodesTable,
+      ElectrodeTable,
+      "electrodes/table",
+      The electrodes table retrieved from the object referenced in the 
+      `electrodes / table` attribute.)
+
 private:
   /**
    * @brief The number of samples already written per channel.

diff --git a/tests/testEcephys.cpp b/tests/testEcephys.cpp
@@ -244,10 +244,11 @@ TEST_CASE("ElectricalSeries", "[ecephys]")
     REQUIRE(readElectrodesDescriptionValues
             == "the electrodes that generated this electrical series");
 
-    // TODO - add test for reading when references are supported in read
-    // auto readElectrodesTableWrapper =
-    // readElectricalSeries->readElectrodesTable(); auto
-    // readElectrodesTableValues = readElectrodesTableWrapper->values().data[0];
+    // Read the references to the ElectrodeTable
+    auto readElectrodesTable = readElectricalSeries->readElectrodesTable();
+    REQUIRE(readElectrodesTable != nullptr);
+    REQUIRE(readElectrodesTable->getPath()
+            == AQNWB::NWB::ElectrodeTable::electrodeTablePath);
   }
 }
 

diff --git a/tests/testHDF5IO.cpp b/tests/testHDF5IO.cpp
@@ -636,10 +636,56 @@ TEST_CASE("HDF5IO; create attributes", "[hdf5io]")
     REQUIRE(hdf5io.objectExists("/data/link"));
   }
 
-  // reference
+  // reference attribute tests
   SECTION("reference")
   {
-    // TODO
+    // Create target objects that we'll reference
+    hdf5io.createGroup("/referenceTargetGroup");
+    hdf5io.createArrayDataSet(BaseDataType::I32,
+                              SizeArray {3},
+                              SizeArray {3},
+                              "/referenceTargetDataset");
+
+    // Test reference to a group
+    SECTION("reference to group")
+    {
+      hdf5io.createReferenceAttribute(
+          "/referenceTargetGroup", "/data", "groupRefAttr");
+      REQUIRE(hdf5io.attributeExists("/data/groupRefAttr"));
+
+      std::string resolvedPath =
+          hdf5io.readReferenceAttribute("/data/groupRefAttr");
+      REQUIRE(resolvedPath == "/referenceTargetGroup");
+    }
+
+    // Test reference to a dataset
+    SECTION("reference to dataset")
+    {
+      hdf5io.createReferenceAttribute(
+          "/referenceTargetDataset", "/data", "datasetRefAttr");
+      REQUIRE(hdf5io.attributeExists("/data/datasetRefAttr"));
+
+      std::string resolvedPath =
+          hdf5io.readReferenceAttribute("/data/datasetRefAttr");
+      REQUIRE(resolvedPath == "/referenceTargetDataset");
+    }
+
+    // Test reading non-existent reference attribute
+    SECTION("non-existent reference attribute")
+    {
+      REQUIRE_THROWS_AS(
+          hdf5io.readReferenceAttribute("/data/nonExistentRefAttr"),
+          std::invalid_argument);
+    }
+
+    // Test reading an attribute that is not a reference attribute
+    SECTION("non-reference attribute")
+    {
+      const int data = 1;
+      hdf5io.createAttribute(BaseDataType::I32, &data, "/data", "intAttr");
+      REQUIRE_THROWS_AS(hdf5io.readReferenceAttribute("/data/intAttr"),
+                        std::invalid_argument);
+    }
   }
 
   // close file