Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
689d747
Initial plan
Copilot Aug 13, 2026
54fd124
Apply remaining changes
Copilot Aug 13, 2026
b5575c7
Make Subject required by default in NWBFile::initialize()
Copilot Aug 13, 2026
b4bd01e
Added Subject type
oruebel Aug 13, 2026
ac90401
Merge branch 'copilot/make-subject-required-by-default' of https://gi…
oruebel Aug 13, 2026
d5a741e
Update unit tests
oruebel Aug 13, 2026
a526662
Update Changelog and fix docstring
oruebel Aug 13, 2026
b732aea
Add missing testSubject.cpp file
oruebel Aug 13, 2026
dabddc7
Fix nwb-inspector validation errors
oruebel Aug 13, 2026
cbd9a1f
Fix validation tests
oruebel Aug 13, 2026
10de7dd
Fix nwb-inspector validation errors
oruebel Aug 13, 2026
bcb223e
Fix nwb-inspector validation errors
oruebel Aug 13, 2026
d3f1893
Fix subject metadata nwb-inspector test
oruebel Aug 13, 2026
49f9ea6
Potential fix for pull request finding
oruebel Aug 13, 2026
b016125
Potential fix for pull request finding
oruebel Aug 13, 2026
7d16947
Potential fix for pull request finding
oruebel Aug 13, 2026
7bb903e
Address review comment
oruebel Aug 13, 2026
e6427bd
Avoid overwriting exiting subject
oruebel Aug 13, 2026
5e39cd2
Prevent invalid Subject data write
oruebel Aug 13, 2026
76be639
Merge branch 'main' into copilot/make-subject-required-by-default
oruebel Aug 16, 2026
597be22
Make Subject constructor protected
oruebel Aug 20, 2026
8cd3833
Merge branch 'copilot/make-subject-required-by-default' of https://gi…
oruebel Aug 20, 2026
88c3ce9
Merge branch 'schema_2_10' into copilot/make-subject-required-by-default
oruebel Aug 20, 2026
0a430ce
Fix missing subject in unit test from parent branch
oruebel Aug 20, 2026
5dab595
Added missing convenience Subject::create to fix the path
oruebel Aug 20, 2026
425ff13
Fix codespell
oruebel Aug 20, 2026
b164a7a
Added missing Subject to unit test to fix nwb-inspector error
oruebel Aug 20, 2026
87af098
Fix missing Subject in Event workflow
oruebel Aug 20, 2026
2efab35
Allow function changing in SubjectSpec to ease initalization with opt…
oruebel Aug 20, 2026
bef3132
Fix codespell
oruebel Aug 20, 2026
19a366a
Merge branch 'add_time_intervals' into copilot/make-subject-required-…
oruebel Aug 20, 2026
b28ea09
Add missing Subject to time intervals tests
oruebel Aug 20, 2026
699e4de
Updated CHANGELOG to group related changes for the upcoming release
oruebel Aug 20, 2026
0ad29bb
Merge branch 'add_time_intervals' into copilot/make-subject-required-…
oruebel Aug 20, 2026
3b0005e
Added missing Subject in new unit test
oruebel Aug 20, 2026
d262d8b
Merge branch 'add_time_intervals' into copilot/make-subject-required-…
oruebel Aug 22, 2026
526e6f5
Fix failing unit test
oruebel Aug 22, 2026
6d04453
Fix NWB scherma version in change log
oruebel Aug 22, 2026
1f64903
Fix column docstrings in EventsTable::createDefaultDataSpecs to match…
oruebel Aug 22, 2026
2d43100
Update MeaningsTable.target doc
oruebel Aug 22, 2026
35d2eaf
Merge branch 'add_time_intervals' into copilot/make-subject-required-…
oruebel Aug 23, 2026
907d4f9
Update changelog to fix formatting
oruebel Aug 23, 2026
dd449bb
Fix bad link in CHANGLEOG
oruebel Aug 30, 2026
07deef1
Merge branch 'add_time_intervals' into copilot/make-subject-required-…
oruebel Aug 31, 2026
7c3630c
Updated changelog
oruebel Sep 2, 2026
64ee4a6
Update changelog
oruebel Sep 2, 2026
59a9718
Merge branch 'add_time_intervals' into copilot/make-subject-required-…
oruebel Sep 2, 2026
616b39e
Ensure cached registered objects match their concrete on-disk type, keep
oruebel Sep 2, 2026
5131191
Fix status checks in the Subject class
oruebel Sep 2, 2026
8fcb541
Fix  NWBFile::initialize()  docstring to clarify subject creation
oruebel Sep 2, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 --ignore=check_electrodes_location_allen_ccf --json-file-path out.json
Comment thread
oruebel marked this conversation as resolved.
if ! grep -q '"messages": \[\]' out.json; then
echo "NWBInspector found issues in the NWB files"
exit 1
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/upgrade_schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 --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
Expand Down
98 changes: 57 additions & 41 deletions CHANGELOG.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ add_library(
src/nwb/event/DurationVectorData.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
Expand Down
13 changes: 13 additions & 0 deletions docs/pages/devdocs/read_design.dox
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,12 @@
* - The \ref AQNWB::NWB::RegisteredType::create "RegisteredType::create" method is used to create
* an instance of a registered subclass by name. This method looks up the subclass name in the
* registry and calls the corresponding factory function to create an instance.
* - Each I/O object caches one canonical registered object per file path.
* The path-only \ref AQNWB::NWB::RegisteredType::create "create" overload
* resolves the object's on-disk ``namespace`` and ``neurodata_type`` before
* reusing a cache entry. A request whose registered type conflicts with an
* existing cache entry fails rather than returning a different type or
* creating a second object for the same path.
*
* 5. **Class Name and Namespace Retrieval**:
* - The \ref AQNWB::NWB::RegisteredType::getTypeName "getTypeName" and
Expand Down Expand Up @@ -339,6 +345,13 @@
* with the type already set. The same approach is also applied in the case of \ref AQNWB::NWB::Data "Data" and its derived
* class \ref AQNWB::NWB::DataTyped "DataTyped".
*
* ``DataTyped<DTYPE>``, ``VectorDataTyped<DTYPE>``, and
* ``NWBDataTyped<DTYPE>`` are typed read facades, not canonical registered
* objects. They are intentionally not stored in
* \ref AQNWB::IO::RecordingObjects "RecordingObjects"; use their specialized
* ``create`` or ``from...`` methods to obtain a typed view of a dataset without
* changing the canonical object cached for its path.
*
* For further details and alternative approaches for implementing templated \ref AQNWB::NWB::RegisteredType "RegisteredType"
* classes, see \ref implement_templated_registered_type.
*
Expand Down
21 changes: 14 additions & 7 deletions docs/pages/devdocs/record_design.dox
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,20 @@
* facilitate reliable memory management.
*
* \note
* Registration with the \ref AQNWB::IO::RecordingObjects "RecordingObjects" of the I/O occurs when a
* \ref AQNWB::NWB::RegisteredType "RegisteredType" is being created via the static
* \ref AQNWB::NWB::RegisteredType::create "RegisteredType::create" factory methods.
* These factory methods also automatically cache and reuse existing objects that are registered in
* \ref AQNWB::IO::RecordingObjects "RecordingObjects". Calling `create` multiple times
* for the same I/O and path will return the exact same object instance, ensuring that the state
* of the object is maintained and preventing unintended interactions from duplicate objects.
* Canonical \ref AQNWB::NWB::RegisteredType "RegisteredType" objects created
* through \ref AQNWB::NWB::RegisteredType::create "RegisteredType::create"
* are registered with the I/O's \ref AQNWB::IO::RecordingObjects
* "RecordingObjects". The cache has one canonical registered object per I/O
* and path; repeated compatible factory calls return that same instance. A
* request for a conflicting registered type fails rather than creating a
* second object for the path.
*
* Typed dataset facades such as \ref AQNWB::NWB::DataTyped "DataTyped",
* \ref AQNWB::NWB::VectorDataTyped "VectorDataTyped", and
* \ref AQNWB::NWB::NWBDataTyped "NWBDataTyped" are intentionally transient
* and are not registered with RecordingObjects. They share the I/O's
* BaseRecordingData cache with the canonical object when accessing the same
* dataset.
*
* \section recording_design_further_reading Further Reading
*
Expand Down
31 changes: 19 additions & 12 deletions docs/pages/devdocs/registered_types.dox
Original file line number Diff line number Diff line change
Expand Up @@ -392,19 +392,25 @@
* the \ref DEFINE_REGISTERED_FIELD macro, we can set the data type for read at compile time, simplifying read.
*
* \note
* Since the templacted \ref AQNWB::NWB::VectorDataTyped "VectorDataTyped<DTYPE>" does not call
* \ref REGISTER_SUBCLASS we must manually declare: 1) `friend class AQNWB::NWB::RegisteredType;`
* and 2) `using VectorData::VectorData;` (as protected) to make sure we have access to the
* constructors and define `static std::shared_ptr<VectorDataTyped> create( const std::string& path, std::shared_ptr<AQNWB::IO::BaseIO> io) { return RegisteredType::create<VectorDataTyped>(path, io); }`
* to make sure we have the factory method available.
*
* Since the templated \ref AQNWB::NWB::VectorDataTyped
* "VectorDataTyped<DTYPE>" does not call \ref REGISTER_SUBCLASS, it must
* declare `friend class AQNWB::NWB::RegisteredType;` and inherit the
* protected `VectorData` constructors with `using VectorData::VectorData;`
* so its specialized factory can construct it. That factory must create a
* transient typed view rather than delegate to
* \ref AQNWB::NWB::RegisteredType::create
* "RegisteredType::create<T>", because that factory registers canonical
* objects in \ref AQNWB::IO::RecordingObjects "RecordingObjects".
* `DataTyped<DTYPE>` and `NWBDataTyped<DTYPE>` follow the same transient
* typed-view pattern; only their non-templated base classes are canonical
* registered types.
*
* \par
* \note
* The `std::unique_ptr<..>` template type is not covariant, i.e., `std::unique_ptr<DerivedClass>` does
* not automatically convert to `std::unique_ptr<BaseClass>`. I.e., while \ref AQNWB::NWB::VectorDataTyped "VectorDataTyped<DTYPE>"
* can be used anywhere \ref AQNWB::NWB::VectorData "VectorData" is being used, when using `std::unique_ptr<..>`
* we cannot rely on the compiler to automatically upcast for us, but we will need to explicitly release
* and upcast `std::unique_ptr<VectorDataTyped<DTYPE>` if `std::unique_ptr<VectorData>` is required. Since
* defining the `DTYPE` is primarily useful for read, we therefore typically use
* AqNWB uses `std::shared_ptr` for registered and typed-view objects, so a
* `std::shared_ptr<VectorDataTyped<DTYPE>>` converts to
* `std::shared_ptr<VectorData>` when needed. Since defining `DTYPE` is
* primarily useful for read, we therefore typically use
* \ref AQNWB::NWB::VectorDataTyped "VectorDataTyped<DTYPE>" on read while
* using \ref AQNWB::NWB::VectorData "VectorData" otherwise.
*
Expand Down Expand Up @@ -532,6 +538,7 @@
* than the requested version. In practice, this is mainly relevant if you are generating classes for the
* NWB `core` and `hdmf-common` namespaces.
*
* \par
* \note
* When generating the test app via the `--generate-test-app` option we also
* need to generate the schema headers files and save them in the `/spec` folder.
Expand Down
35 changes: 27 additions & 8 deletions src/nwb/NWBFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "nwb/ecephys/SpikeEventSeries.hpp"
#include "nwb/epoch/TimeIntervals.hpp"
#include "nwb/file/ElectrodeGroup.hpp"
#include "nwb/file/Subject.hpp"
#include "nwb/misc/AnnotationSeries.hpp"
#include "spec/NamespaceRegistry.hpp"
#include "spec/core.hpp"
Expand Down Expand Up @@ -50,11 +51,13 @@ NWBFile::NWBFile(const std::string& path, std::shared_ptr<IO::BaseIO> 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)
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<AQNWB::NWB::Subject::SubjectSpec>& subjectSpec)
{
auto ioPtr = getIO();
if (!ioPtr) {
Expand Down Expand Up @@ -89,16 +92,31 @@ 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);
return createStatus;
} else {
return Status::Success;
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");
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;
}

bool NWBFile::isInitialized() const
Expand Down Expand Up @@ -200,6 +218,7 @@ Status NWBFile::createFileStructure(const std::string& identifierText,
ioPtr->createStringDataSet("/timestamps_reference_time",
timestampsReferenceTime);
ioPtr->createStringDataSet("/identifier", identifierText);

return Status::Success;
}

Expand Down
9 changes: 8 additions & 1 deletion src/nwb/NWBFile.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "nwb/epoch/TimeIntervals.hpp"
#include "nwb/event/EventsTable.hpp"
#include "nwb/file/ElectrodesTable.hpp"
#include "nwb/file/Subject.hpp"
#include "spec/core.hpp"

/*!
Expand Down Expand Up @@ -114,12 +115,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 subjectSpec Optional subject metadata. Defaults to @c std::nullopt,
* so no Subject group is created unless a SubjectSpec is supplied. For DANDI
* validation, the supplied SubjectSpec must include subjectId, species, sex,
* and either age or dateOfBirth.
*/
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<AQNWB::NWB::Subject::SubjectSpec>&
subjectSpec = std::nullopt);

/**
* @brief Check if the NWB file is initialized.
Expand Down
20 changes: 14 additions & 6 deletions src/nwb/RegisteredType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,14 @@ std::shared_ptr<AQNWB::NWB::RegisteredType> RegisteredType::create(
// Check if the object already exists in the cache
auto existing = getExistingRecordingObject(path, io);
if (existing) {
return existing;
if (existing->getFullTypeName() == fullClassName) {
return existing;
}
std::cerr << "RegisteredType::create: cached object at path " << path
<< " has type " << existing->getFullTypeName()
<< ", which does not match the requested type " << fullClassName
<< "." << std::endl;
return nullptr;
}

// Look up the factory RegisteredType for the fullClassName the registry
Expand Down Expand Up @@ -131,14 +138,15 @@ std::shared_ptr<AQNWB::NWB::RegisteredType> RegisteredType::create(
std::shared_ptr<IO::BaseIO> io,
bool fallbackToBase)
{
// Check if the object already exists in the cache
auto existing = getExistingRecordingObject(path, io);
if (existing) {
return existing;
if (!io) {
std::cerr << "RegisteredType::create: IO object is not available."
<< std::endl;
return nullptr;
}

try {
// Read the "neurodata_type" attribute
// Resolve the on-disk type before consulting the cache so a generic
// wrapper cannot be returned for a derived registered type.
std::string fullClassName = io->getFullTypeName(path);
// Create an instance of the corresponding RegisteredType subclass
return AQNWB::NWB::RegisteredType::create(
Expand Down
37 changes: 26 additions & 11 deletions src/nwb/RegisteredType.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ class RegisteredType : public std::enable_shared_from_this<RegisteredType>
getFactoryMap();

/**
* @brief Create an instance of a registered subclass by name.
* @brief Create or retrieve the canonical instance of a registered subclass.
*
* Each I/O object caches at most one canonical RegisteredType per path. If a
* cached instance exists, its full registered type must match fullClassName;
* otherwise this function logs the mismatch and returns nullptr.
*
* @param fullClassName The combined namespace and class name to instantiate,
* i.e., namespace::class
Expand All @@ -211,8 +215,8 @@ class RegisteredType : public std::enable_shared_from_this<RegisteredType>
* m_defaultUnregisteredGroupTypeClass and
* m_defaultUnregisteredDatasetTypeClass depending on whether the type is a
* group or dataset.
* @return A unique_ptr to the created instance of the subclass, or nullptr if
* the subclass is not found.
* @return The canonical instance of the subclass, or nullptr if the subclass
* is not found or a cached instance has a different registered type.
*/
static std::shared_ptr<RegisteredType> create(
const std::string& fullClassName,
Expand All @@ -221,13 +225,14 @@ class RegisteredType : public std::enable_shared_from_this<RegisteredType>
bool fallbackToBase = false);

/**
* @brief Factory method to create an instance of a subclass of RegisteredType
* from file
* @brief Create or retrieve the canonical RegisteredType instance from file.
*
* The function: 1) reads the "namespace" and "neurodata_type" attributes at
* the given path, 2) looks up the corresponding subclass of RegisteredType
* for that type in the type registry 3) instantiates the subclass to
* represent the object at the path.
* represent the object at the path. The on-disk type is resolved before a
* cached object is used, ensuring that a cached instance has the same
* registered type as the file object.
*
* @param path The path of the registered type.
* @param io A shared pointer to the IO object.
Expand All @@ -237,22 +242,28 @@ class RegisteredType : public std::enable_shared_from_this<RegisteredType>
* m_defaultUnregisteredDatasetTypeClass depending on whether the type is a
* group or dataset.
*
* @return A unique pointer to the created RegisteredType instance, or nullptr
* if creation fails.
* @return The canonical RegisteredType instance, or nullptr if creation
* fails or a cached instance has a different registered type.
*/
static std::shared_ptr<AQNWB::NWB::RegisteredType> create(
const std::string& path,
std::shared_ptr<IO::BaseIO> io,
bool fallbackToBase = false);

/**
* @brief Factory method to create an instance of a subclass of RegisteredType
* by type.
* @brief Create or retrieve a canonical instance of a RegisteredType subtype.
*
* If a cached object exists at path, it is returned only when it can be cast
* to T. A type mismatch is reported and returns nullptr rather than creating
* a second instance for the same path. Non-registered typed facades, such as
* DataTyped and VectorDataTyped, provide their own create methods and are not
* cached by this factory.
*
* @tparam T The subclass of RegisteredType to instantiate.
* @param path The path of the container.
* @param io A shared pointer to the IO object.
* @return A unique_ptr to the created instance of the subclass.
* @return The canonical instance of T, or nullptr if a cached instance has an
* incompatible type.
*/
template<typename T>
static inline std::shared_ptr<T> create(const std::string& path,
Expand All @@ -266,6 +277,10 @@ class RegisteredType : public std::enable_shared_from_this<RegisteredType>
if (casted) {
return casted;
}
std::cerr << "RegisteredType::create: cached object at path " << path
<< " has type " << existing->getFullTypeName()
<< ", which does not match the requested type." << std::endl;
return nullptr;
}
auto result = std::shared_ptr<T>(new T(path, io));
result->registerRecordingObject();
Expand Down
Loading
Loading