From 689d747eefce3c7cb6bcf218d4cab7fb48529da2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Aug 2026 04:49:58 +0000 Subject: [PATCH 01/18] Initial plan From 54fd1242ebcea8d9498a1bf96fa0ec8d1d6a5da5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Aug 2026 04:58:39 +0000 Subject: [PATCH 02/18] Apply remaining changes Co-authored-by: oruebel <10999845+oruebel@users.noreply.github.com> --- .github/workflows/tests.yml | 2 +- .github/workflows/upgrade_schema.yml | 2 +- src/nwb/NWBFile.cpp | 48 ++++++++++++++++++++++++++-- src/nwb/NWBFile.hpp | 42 ++++++++++++++++++++++-- 4 files changed, 87 insertions(+), 7 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c8dde3474..285dffdf9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -144,7 +144,7 @@ jobs: run: | python -m pip install --upgrade pip python -m pip install nwbinspector - nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --ignore=check_subject_exists --json-file-path out.json + nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --json-file-path out.json if ! grep -q '"messages": \[\]' out.json; then echo "NWBInspector found issues in the NWB files" exit 1 diff --git a/.github/workflows/upgrade_schema.yml b/.github/workflows/upgrade_schema.yml index 0f24dedb9..61db0427f 100644 --- a/.github/workflows/upgrade_schema.yml +++ b/.github/workflows/upgrade_schema.yml @@ -58,7 +58,7 @@ jobs: run: | mkdir -p nwb_files cp build/tests/data/*.nwb nwb_files/ - nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --ignore=check_subject_exists --json-file-path out.json + nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --json-file-path out.json if ! grep -q '"messages": \[\]' out.json; then echo "NWBInspector found issues in the NWB files" exit 1 diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index 226a63982..387be20e6 100644 --- a/src/nwb/NWBFile.cpp +++ b/src/nwb/NWBFile.cpp @@ -53,7 +53,8 @@ Status NWBFile::initialize(const std::string& identifierText, const std::string& description, const std::string& dataCollection, const std::string& sessionStartTime, - const std::string& timestampsReferenceTime) + const std::string& timestampsReferenceTime, + const std::optional& subject) { auto ioPtr = getIO(); if (!ioPtr) { @@ -93,7 +94,8 @@ Status NWBFile::initialize(const std::string& identifierText, description, dataCollection, useSessionStartTime, - useTimestampsReferenceTime); + useTimestampsReferenceTime, + subject); return createStatus; } else { return Status::Success; @@ -141,7 +143,8 @@ Status NWBFile::createFileStructure(const std::string& identifierText, const std::string& description, const std::string& dataCollection, const std::string& sessionStartTime, - const std::string& timestampsReferenceTime) + const std::string& timestampsReferenceTime, + const std::optional& subject) { auto ioPtr = getIO(); if (!ioPtr) { @@ -193,6 +196,45 @@ Status NWBFile::createFileStructure(const std::string& identifierText, ioPtr->createStringDataSet("/timestamps_reference_time", timestampsReferenceTime); ioPtr->createStringDataSet("/identifier", identifierText); + + // Create subject group if subject metadata is provided + if (subject.has_value()) { + const std::string subjectPath = + mergePaths(NWBFile::GENERAL_PATH, "subject"); + ioPtr->createGroup(subjectPath); + ioPtr->createAttribute("Subject", subjectPath, "neurodata_type"); + ioPtr->createAttribute("core", subjectPath, "namespace"); + const auto& s = subject.value(); + if (!s.subjectId.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "subject_id"), + s.subjectId); + } + if (!s.species.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "species"), + s.species); + } + if (!s.sex.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "sex"), s.sex); + } + if (!s.age.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "age"), s.age); + } + if (!s.description.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "description"), + s.description); + } + if (!s.genotype.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "genotype"), + s.genotype); + } + if (!s.strain.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "strain"), s.strain); + } + if (!s.weight.empty()) { + ioPtr->createStringDataSet(mergePaths(subjectPath, "weight"), s.weight); + } + } + return Status::Success; } diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index 9029b2aaa..394c331e5 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,35 @@ namespace AQNWB::NWB { +/** + * @brief Metadata about the experimental subject. + * + * All fields are optional. Pass an instance of this struct to + * NWBFile::initialize() to create a Subject group in the NWB file. To + * explicitly record that no subject information is available, pass + * @c std::nullopt for the subject parameter. + */ +struct SubjectMetadata +{ + /// @brief ID of animal/person used/participating in experiment (lab + /// convention). + std::string subjectId; + /// @brief Species of subject. + std::string species; + /// @brief Gender of subject. + std::string sex; + /// @brief Age of subject (e.g., "P90D" for 90 days post-natal). + std::string age; + /// @brief Description of subject and where subject came from. + std::string description; + /// @brief Genetic strain. If absent, assume Wild Type (WT). + std::string genotype; + /// @brief Strain of subject. + std::string strain; + /// @brief Weight at time of experiment. + std::string weight; +}; + /** * @brief The NWBFile class provides an interface for setting up and managing * the NWB file. @@ -98,12 +128,18 @@ class NWBFile : public NWBContainer * time. If empty (default), then the getCurrentTime() will be used. * @param timestampsReferenceTime ISO formatted time string with the timestamp * reference time. If empty (default), then the getCurrentTime() will be used. + * @param subject Optional subject metadata. By default an empty + * SubjectMetadata{} is used, which creates a Subject group in the NWB file. + * Pass @c std::nullopt to explicitly state that no subject should be created + * (e.g., when the subject is unknown). */ Status initialize(const std::string& identifierText, const std::string& description = "a recording session", const std::string& dataCollection = "", const std::string& sessionStartTime = "", - const std::string& timestampsReferenceTime = ""); + const std::string& timestampsReferenceTime = "", + const std::optional& subject = + SubjectMetadata {}); /** * @brief Check if the NWB file is initialized. @@ -256,13 +292,15 @@ class NWBFile : public NWBContainer * time * @param timestampsReferenceTime ISO formatted time string with the timestamp * reference time + * @param subject Optional subject metadata to write to the file. * @return Status The status of the file structure creation. */ Status createFileStructure(const std::string& identifierText, const std::string& description, const std::string& dataCollection, const std::string& sessionStartTime, - const std::string& timestampsReferenceTime); + const std::string& timestampsReferenceTime, + const std::optional& subject); private: /** From b5575c74da577c84f7dbfa46ef9fa7f06c60d7ad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Aug 2026 05:05:29 +0000 Subject: [PATCH 03/18] Make Subject required by default in NWBFile::initialize() - Add SubjectMetadata struct to NWBFile.hpp with optional fields for species, sex, age, description, subjectId, genotype, strain, weight - Add subject parameter to NWBFile::initialize() defaulting to an empty SubjectMetadata{} (creates a Subject group by default); pass std::nullopt to explicitly opt out - Implement subject group creation in createFileStructure() with neurodata_type/namespace attributes for NWB compliance - Update testNWBFile.cpp to expect Subject in findOwnedTypes results - Update workflow example to demonstrate Subject usage with actual data - Remove --ignore=check_subject_exists from tests.yml and upgrade_schema.yml CI workflows Co-authored-by: oruebel <10999845+oruebel@users.noreply.github.com> --- tests/examples/testWorkflowExamples.cpp | 13 ++++++++++++- tests/testNWBFile.cpp | 10 ++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tests/examples/testWorkflowExamples.cpp b/tests/examples/testWorkflowExamples.cpp index ec7135443..5c9a86b08 100644 --- a/tests/examples/testWorkflowExamples.cpp +++ b/tests/examples/testWorkflowExamples.cpp @@ -46,7 +46,18 @@ TEST_CASE("workflowExamples") // [example_workflow_nwbfile_snippet] auto nwbfile = NWB::NWBFile::create(io); - Status initStatus = nwbfile->initialize(generateUuid()); + NWB::SubjectMetadata subject; + subject.subjectId = "mouse001"; + subject.species = "Mus musculus"; + subject.sex = "M"; + subject.age = "P90D"; + subject.description = "Wild type mouse used for electrophysiology study"; + Status initStatus = nwbfile->initialize(generateUuid(), + "a recording session", + "", + "", + "", + subject); AQNWB::checkStatus(initStatus, "NWBFile initialization"); // [example_workflow_nwbfile_snippet] REQUIRE(initStatus == Status::Success); diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index 7977892a2..4346cc53f 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -68,10 +68,11 @@ TEST_CASE("initialize", "[nwb]") REQUIRE(initStatus == Status::Success); REQUIRE(nwbfile->isInitialized()); - // Since we didn't create any typed objects within the NWBFile, we should - // have no owned types + // The default initializes a Subject group so we should have one owned type auto result = nwbfile->findOwnedTypes(); - REQUIRE(result.size() == 0); + REQUIRE(result.size() == 1); + REQUIRE(result.count("/general/subject") == 1); + REQUIRE(result.at("/general/subject") == "core::Subject"); nwbfile->finalize(); // Good practice since we don't call stop recording, but // not essential @@ -268,6 +269,7 @@ TEST_CASE("createElectricalSeries", "[nwb]") } // Check that we can find all the types that we created + // - /general/subject : core::Subject (created by default in initialize) // - /general/extracellular_ephys/array0 : core::ElectrodeGroup // - /general/devices/array1 : core::Device // - /general/extracellular_ephys/electrodes : core::DynamicTable @@ -276,7 +278,7 @@ TEST_CASE("createElectricalSeries", "[nwb]") // - /general/extracellular_ephys/array1 : core::ElectrodeGroup // - /acquisition/esdata0 : core::ElectricalSeries auto result = nwbfile->findOwnedTypes(); - REQUIRE(result.size() == 7); + REQUIRE(result.size() == 8); // finalize the nwb file io->stopRecording(); From b4bd01e9fcb3adf5e9e5265245d2cf37a16f9ca6 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Wed, 12 Aug 2026 23:18:39 -0700 Subject: [PATCH 04/18] Added Subject type --- CMakeLists.txt | 1 + src/nwb/NWBFile.cpp | 73 +++++------------ src/nwb/NWBFile.hpp | 38 +-------- src/nwb/file/Subject.cpp | 152 +++++++++++++++++++++++++++++++++++ src/nwb/file/Subject.hpp | 146 +++++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/testNWBFile.cpp | 4 +- tests/testRegisteredType.cpp | 3 + 8 files changed, 331 insertions(+), 87 deletions(-) create mode 100644 src/nwb/file/Subject.cpp create mode 100644 src/nwb/file/Subject.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 1cb5bed82..ce4330299 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,6 +38,7 @@ add_library( src/nwb/ecephys/SpikeEventSeries.cpp src/nwb/file/ElectrodeGroup.cpp src/nwb/file/ElectrodesTable.cpp + src/nwb/file/Subject.cpp src/nwb/misc/AnnotationSeries.cpp src/nwb/hdmf/base/Container.cpp src/nwb/hdmf/base/Data.cpp diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index 387be20e6..501d064be 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/file/Subject.hpp" #include "nwb/misc/AnnotationSeries.hpp" #include "spec/NamespaceRegistry.hpp" #include "spec/core.hpp" @@ -49,12 +50,13 @@ NWBFile::NWBFile(const std::string& path, std::shared_ptr io) NWBFile::~NWBFile() {} -Status NWBFile::initialize(const std::string& identifierText, - const std::string& description, - const std::string& dataCollection, - const std::string& sessionStartTime, - const std::string& timestampsReferenceTime, - const std::optional& subject) +Status NWBFile::initialize( + const std::string& identifierText, + const std::string& description, + const std::string& dataCollection, + const std::string& sessionStartTime, + const std::string& timestampsReferenceTime, + const std::optional& subjectSpec) { auto ioPtr = getIO(); if (!ioPtr) { @@ -89,17 +91,25 @@ Status NWBFile::initialize(const std::string& identifierText, // Check that the file is empty and initialize if it is bool fileInitialized = isInitialized(); + Status initStatus = Status::Success; if (!fileInitialized) { Status createStatus = createFileStructure(identifierText, description, dataCollection, useSessionStartTime, - useTimestampsReferenceTime, - subject); - return createStatus; - } else { - return Status::Success; + useTimestampsReferenceTime); + initStatus = initStatus && createStatus; } + + // Create subject group and its contents if subject metadata is provided + if (subjectSpec.has_value()) { + const std::string subjectPath = + mergePaths(NWBFile::GENERAL_PATH, "subject"); + auto subject = AQNWB::NWB::Subject::create(subjectPath, ioPtr); + Status subjectInitStatus = subject->initialize(subjectSpec.value()); + initStatus = initStatus && subjectInitStatus; + } + return initStatus; } bool NWBFile::isInitialized() const @@ -143,8 +153,7 @@ Status NWBFile::createFileStructure(const std::string& identifierText, const std::string& description, const std::string& dataCollection, const std::string& sessionStartTime, - const std::string& timestampsReferenceTime, - const std::optional& subject) + const std::string& timestampsReferenceTime) { auto ioPtr = getIO(); if (!ioPtr) { @@ -197,44 +206,6 @@ Status NWBFile::createFileStructure(const std::string& identifierText, timestampsReferenceTime); ioPtr->createStringDataSet("/identifier", identifierText); - // Create subject group if subject metadata is provided - if (subject.has_value()) { - const std::string subjectPath = - mergePaths(NWBFile::GENERAL_PATH, "subject"); - ioPtr->createGroup(subjectPath); - ioPtr->createAttribute("Subject", subjectPath, "neurodata_type"); - ioPtr->createAttribute("core", subjectPath, "namespace"); - const auto& s = subject.value(); - if (!s.subjectId.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "subject_id"), - s.subjectId); - } - if (!s.species.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "species"), - s.species); - } - if (!s.sex.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "sex"), s.sex); - } - if (!s.age.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "age"), s.age); - } - if (!s.description.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "description"), - s.description); - } - if (!s.genotype.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "genotype"), - s.genotype); - } - if (!s.strain.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "strain"), s.strain); - } - if (!s.weight.empty()) { - ioPtr->createStringDataSet(mergePaths(subjectPath, "weight"), s.weight); - } - } - return Status::Success; } diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index 394c331e5..1196cc74e 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -16,6 +16,7 @@ #include "nwb/base/ProcessingModule.hpp" #include "nwb/base/TimeSeries.hpp" #include "nwb/file/ElectrodesTable.hpp" +#include "nwb/file/Subject.hpp" #include "spec/core.hpp" /*! @@ -25,35 +26,6 @@ namespace AQNWB::NWB { -/** - * @brief Metadata about the experimental subject. - * - * All fields are optional. Pass an instance of this struct to - * NWBFile::initialize() to create a Subject group in the NWB file. To - * explicitly record that no subject information is available, pass - * @c std::nullopt for the subject parameter. - */ -struct SubjectMetadata -{ - /// @brief ID of animal/person used/participating in experiment (lab - /// convention). - std::string subjectId; - /// @brief Species of subject. - std::string species; - /// @brief Gender of subject. - std::string sex; - /// @brief Age of subject (e.g., "P90D" for 90 days post-natal). - std::string age; - /// @brief Description of subject and where subject came from. - std::string description; - /// @brief Genetic strain. If absent, assume Wild Type (WT). - std::string genotype; - /// @brief Strain of subject. - std::string strain; - /// @brief Weight at time of experiment. - std::string weight; -}; - /** * @brief The NWBFile class provides an interface for setting up and managing * the NWB file. @@ -138,8 +110,8 @@ class NWBFile : public NWBContainer const std::string& dataCollection = "", const std::string& sessionStartTime = "", const std::string& timestampsReferenceTime = "", - const std::optional& subject = - SubjectMetadata {}); + const std::optional& + subjectSpec = std::nullopt); /** * @brief Check if the NWB file is initialized. @@ -292,15 +264,13 @@ class NWBFile : public NWBContainer * time * @param timestampsReferenceTime ISO formatted time string with the timestamp * reference time - * @param subject Optional subject metadata to write to the file. * @return Status The status of the file structure creation. */ Status createFileStructure(const std::string& identifierText, const std::string& description, const std::string& dataCollection, const std::string& sessionStartTime, - const std::string& timestampsReferenceTime, - const std::optional& subject); + const std::string& timestampsReferenceTime); private: /** diff --git a/src/nwb/file/Subject.cpp b/src/nwb/file/Subject.cpp new file mode 100644 index 000000000..fd4f01b55 --- /dev/null +++ b/src/nwb/file/Subject.cpp @@ -0,0 +1,152 @@ +#include "nwb/file/Subject.hpp" + +#include "Utils.hpp" +#include "nwb/NWBFile.hpp" + +using namespace AQNWB::NWB; +using namespace AQNWB::IO; + +// Initialize the static registered_ member to trigger registration +REGISTER_SUBCLASS_IMPL(Subject) + +Subject::Subject(std::shared_ptr io) + : NWBContainer(mergePaths(NWBFile::GENERAL_PATH, "subject"), io) +{ +} + +// Constructor +Subject::Subject(const std::string& path, std::shared_ptr io) + : NWBContainer(mergePaths(NWBFile::GENERAL_PATH, "subject"), io) +{ + if (path != mergePaths(NWBFile::GENERAL_PATH, "subject")) { + std::cerr << "WARNING: Subject object path must be /general/subject. " + "Ignoring provided path." + << std::endl; + } +} + +// Initialize the object +Status Subject::initialize(const SubjectSpec& subjectSpec) +{ + // Get the IO object` + auto ioPtr = getIO(); + if (!ioPtr) { + std::cerr << "Subject::initialize IO object has been deleted." << std::endl; + return Status::Failure; + } + if (!ioPtr->canModifyObjects()) { + return Status::Failure; + } + + // Call parent initialize method. + Status initStatus = Status::Success; + Status parentInitStatus = NWBContainer::initialize(); + initStatus = initStatus && parentInitStatus; + + // Initialize attributes, datasets, and groups + // Initialize age dataset and age/reference attribute if age is provided + if (subjectSpec.age.has_value()) { + Status ageStatus = ioPtr->createStringDataSet( + mergePaths(this->m_path, "age"), subjectSpec.age.value()); + initStatus = initStatus && ageStatus; + if (!ageStatus) { + std::cerr << "Failed to create age dataset." << std::endl; + } else { + std::string ageReference = subjectSpec.ageReference.value_or("birth"); + Status ageRefStatus = ioPtr->createAttribute( + ageReference, mergePaths(this->m_path, "age"), "reference"); + initStatus = initStatus && ageRefStatus; + if (!ageRefStatus) { + std::cerr << "Failed to create age reference attribute." << std::endl; + } + } + } + + // Initialize date_of_birth dataset if date_of_birth is provided + if (subjectSpec.dateOfBirth.has_value()) { + Status dobStatus = + ioPtr->createStringDataSet(mergePaths(this->m_path, "date_of_birth"), + subjectSpec.dateOfBirth.value()); + initStatus = initStatus && dobStatus; + if (!dobStatus) { + std::cerr << "Failed to create date_of_birth dataset." << std::endl; + } + if (isISO8601Date(subjectSpec.dateOfBirth.value()) == false) { + std::cerr << "Warning: date_of_birth is not in ISO8601 format: " + << subjectSpec.dateOfBirth.value() << std::endl; + } + } + // Initialize description dataset if description is provided + if (subjectSpec.description.has_value()) { + Status descStatus = + ioPtr->createStringDataSet(mergePaths(this->m_path, "description"), + subjectSpec.description.value()); + initStatus = initStatus && descStatus; + if (!descStatus) { + std::cerr << "Failed to create description dataset." << std::endl; + } + } + + // Initialize genotype dataset if genotype is provided + if (subjectSpec.genotype.has_value()) { + Status genotypeStatus = ioPtr->createStringDataSet( + mergePaths(this->m_path, "genotype"), subjectSpec.genotype.value()); + initStatus = initStatus && genotypeStatus; + if (!genotypeStatus) { + std::cerr << "Failed to create genotype dataset." << std::endl; + } + } + + // Initialize sex dataset if sex is provided + if (subjectSpec.sex.has_value()) { + Status sexStatus = ioPtr->createStringDataSet( + mergePaths(this->m_path, "sex"), subjectSpec.sex.value()); + initStatus = initStatus && sexStatus; + if (!sexStatus) { + std::cerr << "Failed to create sex dataset." << std::endl; + } + } + + // Initialize species dataset if species is provided + if (subjectSpec.species.has_value()) { + Status speciesStatus = ioPtr->createStringDataSet( + mergePaths(this->m_path, "species"), subjectSpec.species.value()); + initStatus = initStatus && speciesStatus; + if (!speciesStatus) { + std::cerr << "Failed to create species dataset." << std::endl; + } + } + + // Initialize strain dataset if strain is provided + if (subjectSpec.strain.has_value()) { + Status strainStatus = ioPtr->createStringDataSet( + mergePaths(this->m_path, "strain"), subjectSpec.strain.value()); + initStatus = initStatus && strainStatus; + if (!strainStatus) { + std::cerr << "Failed to create strain dataset." << std::endl; + } + } + + // Initialize subject_id dataset if subject_id is provided + if (subjectSpec.subjectId.has_value()) { + Status subjectIdStatus = ioPtr->createStringDataSet( + mergePaths(this->m_path, "subject_id"), subjectSpec.subjectId.value()); + initStatus = initStatus && subjectIdStatus; + if (!subjectIdStatus) { + std::cerr << "Failed to create subject_id dataset." << std::endl; + } + } + + // Initialize weight dataset if weight is provided + if (subjectSpec.weight.has_value()) { + Status weightStatus = ioPtr->createStringDataSet( + mergePaths(this->m_path, "weight"), subjectSpec.weight.value()); + initStatus = initStatus && weightStatus; + if (!weightStatus) { + std::cerr << "Failed to create weight dataset." << std::endl; + } + } + + // Return the overall status of the initialization + return initStatus; +} diff --git a/src/nwb/file/Subject.hpp b/src/nwb/file/Subject.hpp new file mode 100644 index 000000000..dcd8380b9 --- /dev/null +++ b/src/nwb/file/Subject.hpp @@ -0,0 +1,146 @@ +#pragma once + +// Common STL includes +#include +#include +#include +#include +// Base AqNWB includes for IO and RegisteredType +#include "io/BaseIO.hpp" +#include "io/ReadIO.hpp" +#include "nwb/RegisteredType.hpp" +// Include for parent type +#include "nwb/base/NWBContainer.hpp" +// Include for the namespace schema header +#include "spec/core.hpp" + +namespace AQNWB::NWB +{ + +/** + * @brief Information about the animal or person from which the data was + * measured. + */ +class Subject : public AQNWB::NWB::NWBContainer +{ +public: + /** + * @brief Metadata about the experimental subject. + * + * All fields are optional. Pass an instance of this struct to + * Subject::initialize() to create a Subject group in the NWB file. + */ + struct SubjectSpec + { + /// @brief Age of subject (e.g., "P90D" for 90 days post-natal). + std::optional age = std::nullopt; + /// @brief Age is with reference to this event. Can be ‘birth’ or + /// ‘gestational’. If reference is omitted, ‘birth’ is implied. + std::optional ageReference = std::nullopt; + /// @brief Date of birth of subject as iso formatted date string + std::optional dateOfBirth = std::nullopt; + /// @brief Description of subject and where subject came from. + std::optional description = std::nullopt; + /// @brief Genetic strain. If absent, assume Wild Type (WT). + std::optional genotype = std::nullopt; + /// @brief Biological sex of subject. + std::optional sex = std::nullopt; + /// @brief Species of subject. + std::optional species = std::nullopt; + /// @brief Strain of subject. + std::optional strain = std::nullopt; + /// @brief ID of animal/person used/participating in experiment (lab + /// convention). + std::optional subjectId = std::nullopt; + /// @brief Weight at time of experiment. + std::optional weight = std::nullopt; + }; + + /** + * @brief Constructor for NWBFile class. + * @param io The shared pointer to the IO object. + */ + explicit Subject(std::shared_ptr io); + + /** + * @brief Constructor + * @param path Path to the object in the file + * @param io IO object for reading/writing + */ + Subject(const std::string& path, std::shared_ptr io); + + /** + * @brief Virtual destructor. + */ + virtual ~Subject() override {} + + // TODO: Update the initialize method as appropriate. + /** + * @brief Initialize the object + * @param subjectSpec The SubjectSpec object with the subject metadata + * @return Status::Success if successful, otherwise Status::Failure. + */ + Status initialize(const SubjectSpec& subjectSpec); + + // Define read methods + DEFINE_DATASET_FIELD( + readAge, + recordAge, + std::string, + "age", + "Age of subject. Can be supplied instead of date_of_birth.") + + DEFINE_DATASET_FIELD( + readDateOfBirth, + recordDateOfBirth, + std::string, + "date_of_birth", + "Date of birth of subject. Can be supplied instead of age.") + + DEFINE_DATASET_FIELD(readDescription, + recordDescription, + std::string, + "description", + "Description of subject and where subject came from " + "(e.g. - breeder - if animal).") + + DEFINE_DATASET_FIELD(readGenotype, + recordGenotype, + std::string, + "genotype", + "Genetic strain. If absent - assume Wild Type (WT).") + + DEFINE_DATASET_FIELD( + readSex, recordSex, std::string, "sex", "Gender of subject.") + + DEFINE_DATASET_FIELD( + readSpecies, recordSpecies, std::string, "species", "Species of subject.") + + DEFINE_DATASET_FIELD( + readStrain, recordStrain, std::string, "strain", "Strain of subject.") + + DEFINE_DATASET_FIELD( + readSubjectId, + recordSubjectId, + std::string, + "subject_id", + "ID of animal/person used/participating in experiment (lab convention).") + + DEFINE_DATASET_FIELD(readWeight, + recordWeight, + std::string, + "weight", + "Weight at time of experiment - at time of surgery and " + "at other important times.") + + DEFINE_ATTRIBUTE_FIELD( + readAgeReference, + std::string, + "age/reference", + "Age is with reference to this event. Can be birth or gestational. If " + "reference is omitted - birth is implied.") + + REGISTER_SUBCLASS(Subject, NWBContainer, AQNWB::SPEC::CORE::namespaceName) +}; + +} // namespace AQNWB::NWB diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c599ac3df..fcd4edcaa 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -31,6 +31,7 @@ add_executable(aqnwb_test testRecordingWorkflow.cpp testRecordingObjects.cpp testRegisteredType.cpp + testSubject.cpp testTimeSeries.cpp testTypes.cpp testUtilsFunctions.cpp diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index 7977892a2..afeb61f39 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -542,8 +542,8 @@ TEST_CASE("testAttributeAndDatasetFields", "[nwb]") #else std::time_t utc_epoch = timegm(&parsed); #endif - long offset_total = - ((long)offset_h * 3600 + offset_m * 60) * (offset_sign == '-' ? -1 : 1); + long offset_total = (static_cast(offset_h) * 3600 + offset_m * 60) + * (offset_sign == '-' ? -1 : 1); // Convert local time to UTC by subtracting the offset utc_epoch -= offset_total; auto written_tp = std::chrono::system_clock::from_time_t(utc_epoch); diff --git a/tests/testRegisteredType.cpp b/tests/testRegisteredType.cpp index b8bee3343..1cc621f00 100644 --- a/tests/testRegisteredType.cpp +++ b/tests/testRegisteredType.cpp @@ -84,6 +84,9 @@ TEST_CASE("RegisterType", "[base]") } else if (subclassFullName == "core::ElectrodesTable") { examplePath = ElectrodesTable::electrodesTablePath; exampleName = "electrodes"; + } else if (subclassFullName == "core::Subject") { + examplePath = "/general/subject"; + exampleName = "subject"; } else { examplePath = "/example/path"; exampleName = "path"; From d5a741eaee3b4d14c09fdda05e2f78b84848979f Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Wed, 12 Aug 2026 23:33:07 -0700 Subject: [PATCH 05/18] Update unit tests --- tests/examples/testWorkflowExamples.cpp | 23 +++++++++++++---------- tests/testNWBFile.cpp | 21 ++++++++++++++------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/tests/examples/testWorkflowExamples.cpp b/tests/examples/testWorkflowExamples.cpp index 5c9a86b08..99c10fe9c 100644 --- a/tests/examples/testWorkflowExamples.cpp +++ b/tests/examples/testWorkflowExamples.cpp @@ -13,6 +13,7 @@ #include "nwb/NWBFile.hpp" #include "nwb/ecephys/ElectricalSeries.hpp" #include "nwb/file/ElectrodesTable.hpp" +#include "nwb/file/Subject.hpp" #include "testUtils.hpp" using namespace AQNWB; @@ -46,18 +47,20 @@ TEST_CASE("workflowExamples") // [example_workflow_nwbfile_snippet] auto nwbfile = NWB::NWBFile::create(io); - NWB::SubjectMetadata subject; - subject.subjectId = "mouse001"; - subject.species = "Mus musculus"; - subject.sex = "M"; - subject.age = "P90D"; - subject.description = "Wild type mouse used for electrophysiology study"; + AQNWB::NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.subjectId = "mouse001"; + subjectSpec.species = "Mus musculus"; + subjectSpec.sex = "M"; + subjectSpec.age = "P90D"; + subjectSpec.description = + "Wild type mouse used for electrophysiology study"; + std::string currentTime = getCurrentTime(); Status initStatus = nwbfile->initialize(generateUuid(), "a recording session", - "", - "", - "", - subject); + "data collection info", + currentTime, + currentTime, + subjectSpec); AQNWB::checkStatus(initStatus, "NWBFile initialization"); // [example_workflow_nwbfile_snippet] REQUIRE(initStatus == Status::Success); diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index a8d406cb6..0a0b61fd2 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -13,6 +13,7 @@ #include "nwb/NWBFile.hpp" #include "nwb/base/TimeSeries.hpp" #include "nwb/ecephys/SpikeEventSeries.hpp" +#include "nwb/file/Subject.hpp" #include "nwb/misc/AnnotationSeries.hpp" #include "spec/core.hpp" #include "testUtils.hpp" @@ -68,12 +69,6 @@ TEST_CASE("initialize", "[nwb]") REQUIRE(initStatus == Status::Success); REQUIRE(nwbfile->isInitialized()); - // The default initializes a Subject group so we should have one owned type - auto result = nwbfile->findOwnedTypes(); - REQUIRE(result.size() == 1); - REQUIRE(result.count("/general/subject") == 1); - REQUIRE(result.at("/general/subject") == "core::Subject"); - nwbfile->finalize(); // Good practice since we don't call stop recording, but // not essential io->close(); // close the io @@ -212,7 +207,19 @@ TEST_CASE("createElectricalSeries", "[nwb]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + AQNWB::NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.subjectId = "mouse001"; + subjectSpec.species = "Mus musculus"; + subjectSpec.sex = "M"; + subjectSpec.age = "P90D"; + subjectSpec.description = "Wild type mouse used for electrophysiology study"; + std::string currentTime = getCurrentTime(); + nwbfile->initialize(generateUuid(), + "a recording session", + "data collection info", + currentTime, + currentTime, + subjectSpec); // create the Electrodes Table std::vector mockArrays = getMockChannelArrays(); From a52666271be9ecd83bb0d82975b35a7176458c50 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Wed, 12 Aug 2026 23:43:06 -0700 Subject: [PATCH 06/18] Update Changelog and fix docstring --- CHANGELOG.md | 5 ++++- src/nwb/NWBFile.hpp | 7 +++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8efc7a403..89ffa3716 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * Added `ElectricalSeries::writeAllChannels` method and `IO::writeElectricalSeriesData` overload to simplify zero-copy interleaved multichannel writes. (@copilot, @oruebel, [#293](https://github.com/NeurodataWithoutBorders/aqnwb/pull/293)) * Added `ElectricalSeries::channelsAtSameSampleOffset` method to check if all channels are at the same sample offset, which is a requirement for using `writeAllChannels`. (@copilot, @oruebel, [#293](https://github.com/NeurodataWithoutBorders/aqnwb/pull/293)) - +* Added `Subject` class to represent the `/general/subject` group in NWB files, with corresponding `SubjectSpec` for initialization. (@copilot, @oruebel, [#320](https://github.com/NeurodataWithoutBorders/aqnwb/pull/320)) + * Updated nwbinspector tests to remove `--ignore=check_subject_exists` option to require subject (@oruebel, [#320](https://github.com/NeurodataWithoutBorders/aqnwb/pull/320)) + * Updated `NWBFile::initialize` to accept a `SubjectSpec` argument for subject metadata initialization (@oruebel, [#320](https://github.com/NeurodataWithoutBorders/aqnwb/pull/320)) + ### Changed * **[BREAKING]** Moved `disableSWMRMode` option from `HDF5IO` constructor to a new `HDF5IO::startRecording(bool disableSWMRMode)` overload. The `BaseIO`-compliant `startRecording()` override is preserved and defaults to SWMR enabled. * **Migration Note**: Code using `HDF5IO(path, true)` must be updated to `HDF5IO(path)` followed by `startRecording(true)`. When the `HDF5IO` object is held as a `std::shared_ptr` (e.g., from `createIO`), downcast with `std::dynamic_pointer_cast` to access the overload. (@oruebel [#297](https://github.com/NeurodataWithoutBorders/aqnwb/pull/297)) diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index 1196cc74e..e87cf05a9 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -100,10 +100,9 @@ class NWBFile : public NWBContainer * time. If empty (default), then the getCurrentTime() will be used. * @param timestampsReferenceTime ISO formatted time string with the timestamp * reference time. If empty (default), then the getCurrentTime() will be used. - * @param subject Optional subject metadata. By default an empty - * SubjectMetadata{} is used, which creates a Subject group in the NWB file. - * Pass @c std::nullopt to explicitly state that no subject should be created - * (e.g., when the subject is unknown). + * @param subjectSpec Optional subject metadata. Pass @c std::nullopt + * (default)to explicitly state that no subject should be created (e.g., when + * the subject is unknown). */ Status initialize(const std::string& identifierText, const std::string& description = "a recording session", From b732aeaf3f6002fefed779f45623027d3279039a Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Wed, 12 Aug 2026 23:45:16 -0700 Subject: [PATCH 07/18] Add missing testSubject.cpp file --- tests/testSubject.cpp | 125 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 tests/testSubject.cpp diff --git a/tests/testSubject.cpp b/tests/testSubject.cpp new file mode 100644 index 000000000..22935eb68 --- /dev/null +++ b/tests/testSubject.cpp @@ -0,0 +1,125 @@ +#include +#include +#include + +#include + +#include "Utils.hpp" +#include "io/BaseIO.hpp" +#include "io/hdf5/HDF5IO.hpp" +#include "nwb/NWBFile.hpp" +#include "nwb/file/Subject.hpp" +#include "testUtils.hpp" + +using namespace AQNWB; + +TEST_CASE("Subject", "[file]") +{ + SECTION("is registered and always uses the canonical subject path") + { + auto registry = NWB::RegisteredType::getRegistry(); + REQUIRE(registry.find("core::Subject") != registry.end()); + + auto io = createIO("HDF5", getTestFilePath("subject_constructor.nwb")); + auto subject = NWB::Subject::create("/not/the/subject/path", io); + + REQUIRE(subject->getPath() == "/general/subject"); + REQUIRE(subject->getIO() == io); + } + + SECTION("writes and reads every metadata field") + { + const std::string filename = getTestFilePath("subject_all_fields.nwb"); + auto io = std::make_shared(filename); + io->open(); + io->createGroup("/general"); + + NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.age = "P90D"; + subjectSpec.ageReference = "gestational"; + subjectSpec.dateOfBirth = "2024-01-15T00:00:00.000000+00:00"; + subjectSpec.description = "Test subject"; + subjectSpec.genotype = "wt/wt"; + subjectSpec.sex = "M"; + subjectSpec.species = "Mus musculus"; + subjectSpec.strain = "C57BL/6J"; + subjectSpec.subjectId = "subject-001"; + subjectSpec.weight = "25 g"; + + auto subject = NWB::Subject::create("/general/subject", io); + REQUIRE(subject->initialize(subjectSpec) == Status::Success); + io->close(); + + auto readIO = std::make_shared(filename); + readIO->open(IO::FileMode::ReadOnly); + auto readSubject = std::dynamic_pointer_cast( + NWB::RegisteredType::create("/general/subject", readIO)); + + REQUIRE(readSubject != nullptr); + REQUIRE(readSubject->readAge()->values().data + == std::vector {"P90D"}); + REQUIRE(readSubject->readAgeReference()->values().data + == std::vector {"gestational"}); + REQUIRE(readSubject->readDateOfBirth()->values().data + == std::vector {"2024-01-15T00:00:00.000000+00:00"}); + REQUIRE(readSubject->readDescription()->values().data + == std::vector {"Test subject"}); + REQUIRE(readSubject->readGenotype()->values().data + == std::vector {"wt/wt"}); + REQUIRE(readSubject->readSex()->values().data + == std::vector {"M"}); + REQUIRE(readSubject->readSpecies()->values().data + == std::vector {"Mus musculus"}); + REQUIRE(readSubject->readStrain()->values().data + == std::vector {"C57BL/6J"}); + REQUIRE(readSubject->readSubjectId()->values().data + == std::vector {"subject-001"}); + REQUIRE(readSubject->readWeight()->values().data + == std::vector {"25 g"}); + + readIO->close(); + } + + SECTION("uses birth as the default age reference and omits unset fields") + { + const std::string filename = getTestFilePath("subject_optional_fields.nwb"); + auto io = std::make_shared(filename); + io->open(); + io->createGroup("/general"); + + NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.age = "P7D"; + auto subject = NWB::Subject::create("/general/subject", io); + + REQUIRE(subject->initialize(subjectSpec) == Status::Success); + REQUIRE(subject->readAgeReference()->values().data + == std::vector {"birth"}); + REQUIRE_FALSE(subject->readSpecies()->exists()); + REQUIRE_FALSE(subject->readDateOfBirth()->exists()); + + io->close(); + } + + SECTION("is created by NWBFile initialization when metadata is supplied") + { + const std::string filename = getTestFilePath("nwbfile_subject.nwb"); + auto io = std::make_shared(filename); + io->open(); + + NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.subjectId = "subject-from-nwbfile"; + auto nwbFile = NWB::NWBFile::create(io); + + REQUIRE(nwbFile->initialize(generateUuid(), + "Subject integration test", + "Subject test data collection", + "", + "", + std::optional(subjectSpec)) + == Status::Success); + REQUIRE(io->objectExists("/general/subject")); + REQUIRE(io->objectExists("/general/subject/subject_id")); + + io->close(); + } +} From dabddc7ed4c609e4c62f31582abf36b05c568c4a Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 00:08:21 -0700 Subject: [PATCH 08/18] Fix nwb-inspector validation errors --- tests/testSubject.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/testSubject.cpp b/tests/testSubject.cpp index 22935eb68..9a8e33314 100644 --- a/tests/testSubject.cpp +++ b/tests/testSubject.cpp @@ -20,7 +20,7 @@ TEST_CASE("Subject", "[file]") auto registry = NWB::RegisteredType::getRegistry(); REQUIRE(registry.find("core::Subject") != registry.end()); - auto io = createIO("HDF5", getTestFilePath("subject_constructor.nwb")); + auto io = createIO("HDF5", getTestFilePath("subject_constructor.h5")); auto subject = NWB::Subject::create("/not/the/subject/path", io); REQUIRE(subject->getPath() == "/general/subject"); @@ -29,7 +29,7 @@ TEST_CASE("Subject", "[file]") SECTION("writes and reads every metadata field") { - const std::string filename = getTestFilePath("subject_all_fields.nwb"); + const std::string filename = getTestFilePath("subject_all_fields.h5"); auto io = std::make_shared(filename); io->open(); io->createGroup("/general"); @@ -82,7 +82,7 @@ TEST_CASE("Subject", "[file]") SECTION("uses birth as the default age reference and omits unset fields") { - const std::string filename = getTestFilePath("subject_optional_fields.nwb"); + const std::string filename = getTestFilePath("subject_optional_fields.h5"); auto io = std::make_shared(filename); io->open(); io->createGroup("/general"); @@ -107,14 +107,23 @@ TEST_CASE("Subject", "[file]") io->open(); NWB::Subject::SubjectSpec subjectSpec; - subjectSpec.subjectId = "subject-from-nwbfile"; + subjectSpec.age = "P90D"; + subjectSpec.ageReference = "gestational"; + subjectSpec.dateOfBirth = "2024-01-15T00:00:00.000000+00:00"; + subjectSpec.description = "Test subject"; + subjectSpec.genotype = "wt/wt"; + subjectSpec.sex = "M"; + subjectSpec.species = "Mus musculus"; + subjectSpec.strain = "C57BL/6J"; + subjectSpec.subjectId = "subject-001"; + subjectSpec.weight = "25 g"; + auto currentTime = getCurrentTime(); auto nwbFile = NWB::NWBFile::create(io); - REQUIRE(nwbFile->initialize(generateUuid(), "Subject integration test", "Subject test data collection", - "", - "", + currentTime, + currentTime, std::optional(subjectSpec)) == Status::Success); REQUIRE(io->objectExists("/general/subject")); From cbd9a1f26db12d2868b7881ea985c4040a2b44d0 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 00:34:32 -0700 Subject: [PATCH 09/18] Fix validation tests --- .../examples/test_link_timeseries_example.cpp | 7 ++- tests/testNWBFile.cpp | 45 ++++++++++++++++--- tests/testProcessingModule.cpp | 28 ++++++++++-- tests/testRecordingWorkflow.cpp | 7 ++- tests/testUtils.hpp | 17 +++++++ 5 files changed, 91 insertions(+), 13 deletions(-) diff --git a/tests/examples/test_link_timeseries_example.cpp b/tests/examples/test_link_timeseries_example.cpp index c494e5a4d..ae7d87415 100644 --- a/tests/examples/test_link_timeseries_example.cpp +++ b/tests/examples/test_link_timeseries_example.cpp @@ -21,7 +21,12 @@ TEST_CASE("LinkTimeSeriesExamples", "[timeseries][link]") io->open(); auto nwbfile = NWB::NWBFile::create(io); - auto status = nwbfile->initialize(generateUuid()); + auto status = nwbfile->initialize(generateUuid(), + "Test linked time series", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); REQUIRE(status == Status::Success); // [example_link_timeseries_setup] diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index 0a0b61fd2..04c903efa 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -65,7 +65,12 @@ TEST_CASE("initialize", "[nwb]") REQUIRE(initStatus == Status::Failure); // check that regular init with current times works - initStatus = nwbfile->initialize(generateUuid()); + initStatus = nwbfile->initialize(generateUuid(), + "Test initialized NWB file", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); REQUIRE(initStatus == Status::Success); REQUIRE(nwbfile->isInitialized()); @@ -83,7 +88,12 @@ TEST_CASE("createElectrodesTable", "[nwb]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test electrodes table", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // create the Electrodes Table std::vector mockArrays = getMockChannelArrays(1, 2); @@ -101,7 +111,12 @@ TEST_CASE("createElectricalSeriesWithSubsetOfElectrodes", "[nwb]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test electrical series subset", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // Create electrode table with full set of electrodes (4 channels) std::vector allElectrodes = getMockChannelArrays(4, 1); @@ -151,7 +166,12 @@ TEST_CASE("createElectricalSeriesFailsWithoutElectrodesTable", "[nwb]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test multiple ecephys datasets", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // Attempt to create electrical series without creating electrodes table first std::vector recordingElectrodes = @@ -177,7 +197,12 @@ TEST_CASE("createElectricalSeriesFailsWithOutOfRangeIndices", "[nwb]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test annotation series", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // Create electrode table with 2 channels std::vector tableElectrodes = @@ -428,7 +453,12 @@ TEST_CASE("setCanModifyObjectsMode", "[nwb]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - Status initStatus = nwbfile->initialize(generateUuid()); + Status initStatus = nwbfile->initialize(generateUuid(), + "Test recording mode", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); REQUIRE(initStatus == Status::Success); // start recording @@ -480,7 +510,8 @@ TEST_CASE("testAttributeAndDatasetFields", "[nwb]") description, dataCollection, sessionStartTime, - timestampsReferenceTime); + timestampsReferenceTime, + getTestSubjectSpec()); REQUIRE(initStatus == Status::Success); REQUIRE(nwbfile->isInitialized()); diff --git a/tests/testProcessingModule.cpp b/tests/testProcessingModule.cpp index 28e469e5d..93563bf10 100644 --- a/tests/testProcessingModule.cpp +++ b/tests/testProcessingModule.cpp @@ -30,7 +30,12 @@ TEST_CASE("createProcessingModule", "[processingmodule]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test processing module", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // create and initialize a ProcessingModule auto processingModule = nwbfile->createProcessingModule("test_module"); @@ -64,7 +69,12 @@ TEST_CASE("createMultipleProcessingModules", "[processingmodule]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test processing modules", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // create and initialize two ProcessingModules auto module1 = nwbfile->createProcessingModule("module1"); @@ -104,7 +114,12 @@ TEST_CASE("ProcessingModule createNWBDataInterface and readNWBDataInterface", std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test processing module time series", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // create processing module auto processingModule = nwbfile->createProcessingModule("ecephys"); @@ -191,7 +206,12 @@ TEST_CASE("ProcessingModule createDynamicTable and readDynamicTable", std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test processing module dynamic table", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // create processing module auto processingModule = nwbfile->createProcessingModule("analysis_module"); diff --git a/tests/testRecordingWorkflow.cpp b/tests/testRecordingWorkflow.cpp index e52a8a48d..062f4c224 100644 --- a/tests/testRecordingWorkflow.cpp +++ b/tests/testRecordingWorkflow.cpp @@ -46,7 +46,12 @@ TEST_CASE("writeContinuousData", "[recording]") // 3. create NWBFile object auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + nwbfile->initialize(generateUuid(), + "Test continuous recording", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()); // 4. create an electrodes table. nwbfile->createElectrodesTable(mockRecordingArrays); diff --git a/tests/testUtils.hpp b/tests/testUtils.hpp index c3fe3f29d..e57f53ba3 100644 --- a/tests/testUtils.hpp +++ b/tests/testUtils.hpp @@ -11,6 +11,7 @@ #include "Channel.hpp" #include "Types.hpp" #include "io/hdf5/HDF5IO.hpp" +#include "nwb/file/Subject.hpp" using namespace AQNWB; using namespace AQNWB::IO; @@ -49,6 +50,22 @@ inline std::string getTestFilePath(const std::string& filename) return filepath.generic_string(); } +inline NWB::Subject::SubjectSpec getTestSubjectSpec() +{ + NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.age = "P90D"; + subjectSpec.ageReference = "birth"; + subjectSpec.dateOfBirth = "2024-01-15T00:00:00.000000+00:00"; + subjectSpec.description = "Test subject"; + subjectSpec.genotype = "wt/wt"; + subjectSpec.sex = "U"; + subjectSpec.species = "Mus musculus"; + subjectSpec.strain = "C57BL/6J"; + subjectSpec.subjectId = "test-subject"; + subjectSpec.weight = "25 g"; + return subjectSpec; +} + inline std::vector getMockChannelArrays( SizeType numChannels = 2, SizeType numArrays = 2, From 10de7dd4223058f3f1ec9f9aaaff3a1548a6ebf0 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 00:48:20 -0700 Subject: [PATCH 10/18] Fix nwb-inspector validation errors --- tests/testNWBFile.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index 04c903efa..e6b4fcb24 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -394,7 +394,14 @@ TEST_CASE("createAnnotationSeries", "[nwb]") std::make_shared(filename); io->open(); auto nwbfile = NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + auto currentTime = getCurrentTime(); + auto subjectSpec = getTestSubjectSpec(); + nwbfile->initialize(generateUuid(), + "a recording session", + "data collection info", + currentTime, + currentTime, + subjectSpec); // create Annotation Series std::vector mockAnnotationNames = {"annotations1", From bcb223e6a85156ddfcb88d2434171a576db5cab6 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 01:07:22 -0700 Subject: [PATCH 11/18] Fix nwb-inspector validation errors --- .github/workflows/tests.yml | 2 +- .github/workflows/upgrade_schema.yml | 2 +- CHANGELOG.md | 1 + tests/testNWBFile.cpp | 11 ++++++++++- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 285dffdf9..6a06a7584 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -144,7 +144,7 @@ jobs: run: | python -m pip install --upgrade pip python -m pip install nwbinspector - nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --json-file-path out.json + nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --ignore=check_electrodes_location_allen_ccf --json-file-path out.json if ! grep -q '"messages": \[\]' out.json; then echo "NWBInspector found issues in the NWB files" exit 1 diff --git a/.github/workflows/upgrade_schema.yml b/.github/workflows/upgrade_schema.yml index 61db0427f..d32ed0804 100644 --- a/.github/workflows/upgrade_schema.yml +++ b/.github/workflows/upgrade_schema.yml @@ -58,7 +58,7 @@ jobs: run: | mkdir -p nwb_files cp build/tests/data/*.nwb nwb_files/ - nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --json-file-path out.json + nwbinspector nwb_files --threshold BEST_PRACTICE_VIOLATION --ignore=check_electrodes_location_allen_ccf --json-file-path out.json if ! grep -q '"messages": \[\]' out.json; then echo "NWBInspector found issues in the NWB files" exit 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 89ffa3716..3411ed017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **Migration Note**: Code using `HDF5IO(path, true)` must be updated to `HDF5IO(path)` followed by `startRecording(true)`. When the `HDF5IO` object is held as a `std::shared_ptr` (e.g., from `createIO`), downcast with `std::dynamic_pointer_cast` to access the overload. (@oruebel [#297](https://github.com/NeurodataWithoutBorders/aqnwb/pull/297)) ### Fixed +* Updated nwbinspector validation tests to ignore the Allen CCF electrode location check when validating mock electrode locations. (@oruebel, [#320](https://github.com/NeurodataWithoutBorders/aqnwb/pull/320)) * Updated nwbinspector validation tests in the CI to: 1) `--ignore=check_subject_exists` and 2) remove dependency on `sanitizer` tests to speed up CI (@oruebel, [#289](https://github.com/NeurodataWithoutBorders/aqnwb/pull/289)) * Fixed `get_utc_offset_seconds` to correctly account for daylight saving time using platform-specific APIs (`tm_gmtoff` on Unix/macOS; `_get_timezone` + `_get_dstbias` on Windows), preventing `session_start_time` from being written ~1 hour ahead of UTC during DST (@cboulay, [#295](https://github.com/NeurodataWithoutBorders/aqnwb/pull/295)) * Fixed HDF5 string type creation to explicitly use UTF-8 character set for fixed-length and variable-length strings in datasets and attributes, improving compatibility with hdmf/PyNWB string decoding (@copilot, @oruebel [#319](https://github.com/NeurodataWithoutBorders/aqnwb/pull/319)) diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index e6b4fcb24..ca125622a 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -324,7 +325,15 @@ TEST_CASE("createMultipleEcephysDatasets", "[nwb]") std::shared_ptr io = std::make_shared(filename); io->open(); auto nwbfile = AQNWB::NWB::NWBFile::create(io); - nwbfile->initialize(generateUuid()); + NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.species = "Mus musculus"; + auto currentTime = getCurrentTime(); + nwbfile->initialize(generateUuid(), + "a recording session", + "data collection info", + currentTime, + currentTime, + std::optional(subjectSpec)); // create ElectrodesTable std::vector mockArrays = getMockChannelArrays(2, 2); From d3f1893984f06908aa730441430a34bb36d02728 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 01:43:46 -0700 Subject: [PATCH 12/18] Fix subject metadata nwb-inspector test --- tests/testNWBFile.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index ca125622a..e22c4696d 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include @@ -325,15 +324,13 @@ TEST_CASE("createMultipleEcephysDatasets", "[nwb]") std::shared_ptr io = std::make_shared(filename); io->open(); auto nwbfile = AQNWB::NWB::NWBFile::create(io); - NWB::Subject::SubjectSpec subjectSpec; - subjectSpec.species = "Mus musculus"; auto currentTime = getCurrentTime(); nwbfile->initialize(generateUuid(), "a recording session", "data collection info", currentTime, currentTime, - std::optional(subjectSpec)); + getTestSubjectSpec()); // create ElectrodesTable std::vector mockArrays = getMockChannelArrays(2, 2); From 49f9ea635b8bb79660b1aa6e03b27d1678b435b9 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 02:05:24 -0700 Subject: [PATCH 13/18] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/nwb/file/Subject.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwb/file/Subject.hpp b/src/nwb/file/Subject.hpp index dcd8380b9..4104413f3 100644 --- a/src/nwb/file/Subject.hpp +++ b/src/nwb/file/Subject.hpp @@ -57,7 +57,7 @@ class Subject : public AQNWB::NWB::NWBContainer }; /** - * @brief Constructor for NWBFile class. + * @brief Constructor for Subject. * @param io The shared pointer to the IO object. */ explicit Subject(std::shared_ptr io); From b01612571277b50694e95dabdeec418b2f236da6 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 02:05:50 -0700 Subject: [PATCH 14/18] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/nwb/file/Subject.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwb/file/Subject.cpp b/src/nwb/file/Subject.cpp index fd4f01b55..2093b6748 100644 --- a/src/nwb/file/Subject.cpp +++ b/src/nwb/file/Subject.cpp @@ -28,7 +28,7 @@ Subject::Subject(const std::string& path, std::shared_ptr io) // Initialize the object Status Subject::initialize(const SubjectSpec& subjectSpec) { - // Get the IO object` + // Get the IO object auto ioPtr = getIO(); if (!ioPtr) { std::cerr << "Subject::initialize IO object has been deleted." << std::endl; From 7d16947a4f8945b63926b1024c124dfd51113a4b Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 02:06:24 -0700 Subject: [PATCH 15/18] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/nwb/NWBFile.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nwb/NWBFile.hpp b/src/nwb/NWBFile.hpp index e87cf05a9..e8855a1ed 100644 --- a/src/nwb/NWBFile.hpp +++ b/src/nwb/NWBFile.hpp @@ -100,9 +100,9 @@ class NWBFile : public NWBContainer * time. If empty (default), then the getCurrentTime() will be used. * @param timestampsReferenceTime ISO formatted time string with the timestamp * reference time. If empty (default), then the getCurrentTime() will be used. - * @param subjectSpec Optional subject metadata. Pass @c std::nullopt - * (default)to explicitly state that no subject should be created (e.g., when - * the subject is unknown). + * @param subjectSpec Optional subject metadata. By default, an empty Subject + * is created. Pass @c std::nullopt to explicitly state that no subject should + * be created (e.g., when the subject is unknown). */ Status initialize(const std::string& identifierText, const std::string& description = "a recording session", From 7bb903ea41ab6da5d42311c80b2fb7f2c859fd53 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 02:11:32 -0700 Subject: [PATCH 16/18] Address review comment --- src/nwb/NWBFile.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/nwb/NWBFile.cpp b/src/nwb/NWBFile.cpp index 501d064be..9dfed07b1 100644 --- a/src/nwb/NWBFile.cpp +++ b/src/nwb/NWBFile.cpp @@ -105,9 +105,15 @@ Status NWBFile::initialize( if (subjectSpec.has_value()) { const std::string subjectPath = mergePaths(NWBFile::GENERAL_PATH, "subject"); - auto subject = AQNWB::NWB::Subject::create(subjectPath, ioPtr); - Status subjectInitStatus = subject->initialize(subjectSpec.value()); - initStatus = initStatus && subjectInitStatus; + if (!ioPtr->objectExists(subjectPath)) { + auto subject = AQNWB::NWB::Subject::create(subjectPath, ioPtr); + Status subjectInitStatus = subject->initialize(subjectSpec.value()); + initStatus = initStatus && subjectInitStatus; + } else { + std::cerr << "Subject group already exists in the file. Skipping " + "subject initialization." + << std::endl; + } } return initStatus; } From e6427bd4eed8def1ed0b068eb3b637ad3f8156ca Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 02:16:55 -0700 Subject: [PATCH 17/18] Avoid overwriting exiting subject --- tests/testNWBFile.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/testNWBFile.cpp b/tests/testNWBFile.cpp index e22c4696d..783d215b4 100644 --- a/tests/testNWBFile.cpp +++ b/tests/testNWBFile.cpp @@ -79,6 +79,34 @@ TEST_CASE("initialize", "[nwb]") io->close(); // close the io } +TEST_CASE("initialize preserves existing subject metadata", "[nwb]") +{ + const std::string filename = + getTestFilePath("testInitializeExistingSubject.nwb"); + auto io = std::make_shared(filename); + io->open(); + + auto nwbfile = NWB::NWBFile::create(io); + REQUIRE(nwbfile->initialize(generateUuid()) == Status::Success); + + auto existingSubjectSpec = getTestSubjectSpec(); + existingSubjectSpec.subjectId = "existing-subject"; + auto subject = NWB::Subject::create("/general/subject", io); + REQUIRE(subject->initialize(existingSubjectSpec) == Status::Success); + + REQUIRE(nwbfile->initialize(generateUuid(), + "Test initialized NWB file", + "Test data collection", + getCurrentTime(), + getCurrentTime(), + getTestSubjectSpec()) + == Status::Success); + REQUIRE(subject->readSubjectId()->values().data + == std::vector {"existing-subject"}); + + io->close(); +} + TEST_CASE("createElectrodesTable", "[nwb]") { std::string filename = getTestFilePath("createElectrodesTable.nwb"); From 5e39cd2522498c96d79a3bc916f1d37d8aa8b3a4 Mon Sep 17 00:00:00 2001 From: Oliver Ruebel Date: Thu, 13 Aug 2026 02:21:15 -0700 Subject: [PATCH 18/18] Prevent invalid Subject data write --- src/nwb/file/Subject.cpp | 16 +++++++++------- tests/testSubject.cpp | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/nwb/file/Subject.cpp b/src/nwb/file/Subject.cpp index 2093b6748..93067abb3 100644 --- a/src/nwb/file/Subject.cpp +++ b/src/nwb/file/Subject.cpp @@ -64,16 +64,18 @@ Status Subject::initialize(const SubjectSpec& subjectSpec) // Initialize date_of_birth dataset if date_of_birth is provided if (subjectSpec.dateOfBirth.has_value()) { - Status dobStatus = - ioPtr->createStringDataSet(mergePaths(this->m_path, "date_of_birth"), - subjectSpec.dateOfBirth.value()); - initStatus = initStatus && dobStatus; - if (!dobStatus) { - std::cerr << "Failed to create date_of_birth dataset." << std::endl; - } if (isISO8601Date(subjectSpec.dateOfBirth.value()) == false) { std::cerr << "Warning: date_of_birth is not in ISO8601 format: " << subjectSpec.dateOfBirth.value() << std::endl; + initStatus = Status::Failure; + } else { + Status dobStatus = + ioPtr->createStringDataSet(mergePaths(this->m_path, "date_of_birth"), + subjectSpec.dateOfBirth.value()); + initStatus = initStatus && dobStatus; + if (!dobStatus) { + std::cerr << "Failed to create date_of_birth dataset." << std::endl; + } } } // Initialize description dataset if description is provided diff --git a/tests/testSubject.cpp b/tests/testSubject.cpp index 9a8e33314..1cd40ec0e 100644 --- a/tests/testSubject.cpp +++ b/tests/testSubject.cpp @@ -100,6 +100,24 @@ TEST_CASE("Subject", "[file]") io->close(); } + SECTION("rejects invalid date of birth without writing it") + { + const std::string filename = getTestFilePath("subject_invalid_dob.h5"); + auto io = std::make_shared(filename); + io->open(); + io->createGroup("/general"); + + NWB::Subject::SubjectSpec subjectSpec; + subjectSpec.dateOfBirth = "January 15, 2024"; + auto subject = NWB::Subject::create("/general/subject", io); + + REQUIRE(subject->initialize(subjectSpec) == Status::Failure); + REQUIRE_FALSE(subject->readDateOfBirth()->exists()); + REQUIRE_FALSE(io->objectExists("/general/subject/date_of_birth")); + + io->close(); + } + SECTION("is created by NWBFile initialization when metadata is supplied") { const std::string filename = getTestFilePath("nwbfile_subject.nwb");