-
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
Update handling of colnames #149
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #149 +/- ##
==========================================
- Coverage 93.97% 93.93% -0.04%
==========================================
Files 62 64 +2
Lines 4629 4915 +286
Branches 269 272 +3
==========================================
+ Hits 4350 4617 +267
- Misses 269 288 +19
Partials 10 10 ☔ View full report in Codecov by Sentry. |
… and ElectrodeTable
Support overwrite of string array attributes
@stephprince this PR is ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks. I just had a question about the overwriting of variable length strings but all of the electrodes table and column changes look good to me.
@stephprince please take another look. I believe I've addressed your suggestion. |
@stephprince I resolved all merge conflicts with |
This PR updates the handling of the
DynamicTable.colNames
to: 1) avoid the need for specifyingcolNames
at the beginning and 2) to allow us to dynamically add columns to aDynamicTable
(even one that was read from a file)Related issues
Fix #107
Fix #150
Fix #129
Fixed writing of variable-length string attributes and
VectorData
columns:BaseIO.createAttribute
for string arrays to explicitly allow us to force overwrite of existing attributes and updatedHDF5IO.createAttribute
accordingly. HDF5 does not support expanding attributes in the same way it does for datasets so we need to delete the existing attribute and overwrite it with a new one with the same name.HDF5IO.createAttribute
for scalar strings transformed the scalar string to an array of strings and wrote it using the array method. This PR implements the function to write scalar strings.DynamicTable.add_column
to avoid unnecessary iteration and usewriteDataBlock
directlyElectrodeTable
to use variable length strings instead of fixed length stringsImproved handling of
Status
Status
forDynamicTable
functions that perform write operations (e.g,.initialize
,finalize
,add_column
etc.) as well as relevant other classes, e.g.,Data
,VectorData
,Container
...&&
and||
operator for theStatus
enum to simplify combining multipleStatus
checksUpdated handling of
colNames
to correctly handle dynamic adding of columnscolNames
to the end by addingDynamicTable.finalize
akin to the existingElectrodeTable.finalize
. This allows for columns to be added to theDynamicTable
DynamicTable.finalize
to overwrite column names to correctly support appending of columns to existing tablesDynamicTable
constructor to read the existing column names if they exist from the file so that we can append new columns to existing tablesUpdated unit tests
HDF5IO.createAttribute
DynamicTable
ElectrodeTable
unit tests to appropriately check for the status of write operations and fix existing failing tests due to missingDevice
andElectrodeGroup