-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Propose refactor of I/O class organization #88
Labels
category: proposal
proposed enhancements or new features
priority: low
alternative solution already working and/or relevant to only specific user(s)
Comments
oruebel
added
category: proposal
proposed enhancements or new features
priority: low
alternative solution already working and/or relevant to only specific user(s)
labels
Sep 3, 2024
@stephprince what do you think? |
oruebel
added a commit
that referenced
this issue
Sep 5, 2024
oruebel
added a commit
that referenced
this issue
Jan 13, 2025
* Define base classes for reading * Support reading datasets and attributes * Add functions to construct ReadDatasetWrapper and ReadAttributeWrapper from BaseIO * Add test for using the ReadDatasetWrapper * Read ElectricalSeries.data example working * Add example for data read * Add user docs for data read * Update read user docs * Add read software design figure and more details on the design * Implement function to allow us to get the storage object type for a given path * Rename getObjectType to getStroageObjectType * Fix reading of attributes and add TimeSeries.resolution read * Add example for reading an attribute * Add ReadDataWrapperBase as common base class for ReadDatasetWrapper and ReadAttributeWrapper * Remove NWBFile.identifierText property * Allow setting of value type template parameter for ReadDataWrapperBase and subtypes * Save the std::type_index as part of the DataBlock to support runtime checking * Fix #88 move io classes to their own module * Refactor to add IO namespace to match folder structure * Move HDF5RecordingData to its own hpp/cpp files * Moved read I/O classes to new ReadIO.hpp * Refactor the read data wrappers to use a single ReadDataWrapper class for both Attributes and Datasets * Replace redundant ReadObjectType with StorageObjectType * Add traits for ReadObjectWrapper to prevent instantiation for Group and Undefined types * Update ElectrodGroup to provide standard Container constructor and move args to initalize * Register subtypes * Move the registry to a new RegisteredType class to also cover non-Group types, e.g., Data, VectorData etc. * Ensure all neurodata_types are registered correctly * Add unittest for RegisteredTrype * Add developer docs for RegisteredTypes * Update read test to use the registry to construct the class * Fix getTypeName and getNamespace and use them instead of hard-coded values * Add test for reading a TimeSeries Container back * Add RegisteredType::create method to read a type from file * Run test workflows on all PRs * Fix static assert for ReadDataWrapper * Sync add_read with main branch (#101) * Merged main into add_read * Fix docs build for SpikeEventSeries * Fix code formatting * Fix segfault due to duplicate declaration of NWBFile.io parameter * Fix formatting and docs error after merge with main * Fix formatting after merge * Remove SpikeEventSeries.neurodata_type * Register SpikeEventSeries as a type * Updated RegisterTypes to allow overwrite of the typename to use and overwrite the typename for ElectrodeTable * Fix code formatting * Add missing description for DynamicTableRegion * Fix #109 Add missing axis attribute for channel_conversion and remove extra attributes * Remove undefined functions from Device * Rename member variable in ElectricalSeries * Rename member variable in SpikeEventSeries * Added mergePaths functions and made functions in Utils.hpp static inline * Updated ElectrodeTable to use mergePaths method * Updated DynamicTable to use mergePaths method * Update ElectrodeGroup to use mergePaths * Updated ElectricalSeries to use mergePaths * Updated TimerSeries to use mergePaths function * Updated NWBFile to use the mergePaths function * Updated ElectrodeTable static paths to not used trailing / for consistency * Fix Doxygen error by extracting also static members in the docs * Fix section level in install.dox * Update ReadDataWrapper to use m_ member naming convention * Updated DataBlock / DataBlockGeneric to avoid shadowing of constructor args * Fix reference error in read.dox * Add ReadDataWrapper isType, getPath, getIO methods and update docstrings * Create macro DEFINE_FIELD to simplify creating access methods for fields in RegisteredType subclasses * Replace previous field access methods in TimeSeries with DEFINE_FIELD macro calls * Replace TimeSeries.dataLazy and resolutionLazy with DEFINE_FIELD definitions * Update the Doxygen built to expand the DEFINE_FIELD macro to document the generated functions * Fix spellcheck * Fix section levels in read.dox * Add docs for using the DEFINE_FIELD macro * Added attributeExists method on IO and ReadDataWrapper::exists methods to check that an object actually exists * Add TimeSeries.descriptionLazy field to test that reading string attributes works * Added function getGroupObjects to the I/O to allow gettings all objects contained in a group * Add BaseIO::findTypes method to search for neurodata_types in a file * Add example for searching for typed objects * Update ElectrodeTable to use m_ member names * Added field definitions for TimeSeries * Add some design docs about reading registered types * Update read types design figure * Minor update to docs of RegisteredType * Update docs/pages/userdocs/read.dox Co-authored-by: Steph Prince <[email protected]> * Update src/io/ReadIO.hpp Co-authored-by: Steph Prince <[email protected]> * Update src/io/ReadIO.hpp * Rename ReadDataWrapper.is_dataset * Update src/io/hdf5/HDF5IO.hpp Co-authored-by: Steph Prince <[email protected]> * Remove duplicate code in NWBFile * Remove outdated ToDo item * Apply suggestions on docs from code review Co-authored-by: Steph Prince <[email protected]> * Fix build error after merge * Fix build error due to error during merge with base branch * Apply suggestions from code review Co-authored-by: Steph Prince <[email protected]> * Fix read.dox figure based on suggestion * Fix docs based on code review * update getAttribute method to detect object type * update read example * add file mode options and readonly mode to io * update example to use readonly * add getter functions to Container classes with attributes * fix formatting * update lint workflow * add NWBFile fields * update filenames * remove duplicate file mode checks * Fix spelling and complete merge * Fix missing merge in file * Install Boost multi-array in windows action * Use std::vector instead of variable-length arrays to avoid Windows built errors * Add note to docs to clarify non-virtual DEFINE_FIELD * Add HDF5IO tests for add_read PR (#118) * Add unit test for HDFIO::getH5ObjectType * Fix behavior of HDF5IO::getH5ObjectType when object does not exist * Add unit tests for HDF5IO::getNativeType * Add unit test for HDF5IO::getH5Type * Add unit test for HDF5IO::objectExists * Add unit tests for HDF5IO::attributeExists * add unit test for HDF5IO::getGroupObjects * Add unit test for HDF5IO::getStorageObjectType * Fix reading of arrays and select dtypes for HDF5IO::readAttribute * Add initial unit tests for HDF5IO::readAttribute * Fix reading of string array attributes via HDF5IO::readAttribute * Fix reading of reference attributes * Add HDF5IO::readAttribute unit test for invalid and double attribute * Add HDF5IO::readAttriute unit test for reading attribute from a dataset * Fix handling of non-existent file ReadOnly and ReadWrite mode * Add unit tests for HDF5IO::open for file modes * Add additonal unit tests for HDF5IO::readDataset * Fix element selection in HDF5IO::readDataset * Add unit test for reading hyperslap with HDF5IO::readDataset * Fix hyperslap selection with block in HDF5IO::readDataset * Expand unit tests for HDF5IO::readDataset with hyperslaps * Add note with TODO item to the tests * Improve coverage of numeric types in readDataset * Fix determining string size on read * Set string size when creating the dataset * Fix error in commmented unit test * Fix writing of fixed length string datasets * Add test writing fixed length string data * Fix reading of fixed length strings * Fix writing of variable length string datasets * Fix writing of variable length string datasets * Fix reading of variable length string dataset * Fix reading of empty string datasets * Minor enhancement to check for address santizer * Create RecordingData::writeStrinData to simplify writing of strings * Update tests/testHDF5IO.cpp Co-authored-by: Steph Prince <[email protected]> * Update tests/testHDF5IO.cpp Co-authored-by: Steph Prince <[email protected]> * Fix formatting --------- Co-authored-by: Steph Prince <[email protected]> * Update read example * Add missing offset attriubte for TimeSeries.data * Add basic unit test for TimeSeries read * Add more REQUIRE checks * Add missing TimeSerires.conitnuity field for write. Move timeseries-specific write method from BaseIO to TimeSeries * Fix spellcheck errors * Add read test for timestamps.unit and timestamps.interval * Support using starting_time and rate with TimeSeries * Add missing timeseries control and control description. Fix chunking of timestamps * Move neurodata_type and namespace attributes to Container * address validation errors in add_read (#125) * use writeStringDataBlock function * update SES test to use multiple channels * update filename to avoid access errors * fix formatting * Fix #126 Initiallization of VectorData * Renamed testBase to testTimeSeries * Add initial unit tests for VectorData * Add additional unit tests for VectorData * Add write/read test for Data * Add write/read test for ElementIdentifiers * Add unit test for Device * Remove extra comment * Remove extra comments * Update src/io/hdf5/HDF5IO.cpp Co-authored-by: Steph Prince <[email protected]> * Update tests/testTimeSeries.cpp Co-authored-by: Steph Prince <[email protected]> * Move test to RegisteredType * Update writing of control_description * Fix code linting * Add pass-through for control to RecordingContainers::write.. methods * Minor updates to the example * Fix file name collision in tests * Make calling io->open() explicit * Adjust slice to check if it fixes Windows error * Add output to help debug Windows issue * Add debug output in HDF5IO to debug Windows issue * Update NWBFile.hpp * Fix code formatting * Some more debug ouput for Windows * Fix read path to avoid potentially reading from different series * Clean up deug output --------- Co-authored-by: Steph Prince <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
category: proposal
proposed enhancements or new features
priority: low
alternative solution already working and/or relevant to only specific user(s)
src/io
with with the classes from BaseIO.hpphdf5
folder tosrc/io/hdf5
The text was updated successfully, but these errors were encountered: