Fix NixIO writing of datetime array_annotations (#1198)#1875
Open
Leonard013 wants to merge 1 commit into
Open
Conversation
NixIO._write_property passed datetime/date/time objects contained in array_annotations straight to nixio.create_property, which cannot store them and raised "Unknown type for value ...". Convert each datetime item to its ISO string form via dt_to_nix() on write (mirroring the existing scalar-annotation path) and record the annotation type as the property definition. On read, reconstruct multi-element datetime array_annotations element-wise into an object-dtype ndarray so the write->read roundtrip preserves them. Fixes NeuralEnsemble#1198 Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1198.
Why
NixIOcannot writearray_annotationswhose elements aredatetime(ordate/time) objects.neo.coreaccepts such values as array-annotationelements (they are in
ALLOWED_ANNOTATION_TYPES, stored as an object-dtypendarray), so a
Segment/Event/AnalogSignalcarrying them can be built andlives fine in memory — but writing it to a
.nixfile crashes:As @JuliaSprenger pointed out in the issue,
_write_propertymust convert thedatetime objects before saving. Neo already knows how to do this for scalar
datetime annotations via
dt_to_nix()/dt_from_nix(); the iterable(array-annotation) branch just never used them.
What
neo/io/nixio.py, two surgical changes:_write_property, iterable branch): when an item is adatetime/date/time, convert it to its ISO string withdt_to_nix()and record the returned annotation type as the property's
definition—exactly mirroring the existing scalar-annotation path a few lines above.
_nix_attr_to_neo): when a property carries a datetimedefinitionand holds multiple values (alist), reconstruct itelement-wise with
dt_from_nix()into an object-dtype ndarray. A singlevalue keeps the previous scalar behaviour unchanged. The object dtype is
required so the reconstructed annotation is accepted back by
array_annotate()on the read path.This makes datetime array_annotations survive a write → read roundtrip for all
three of
datetime,dateandtime.Tests
Added
NixIOWriteTest.test_datetime_array_annotationsinneo/test/iotest/test_nixio.py(modelled on the neighbouringtest_empty_array_annotations): writes anEventwithdatetime,dateandtimearray_annotations, reads it back, and asserts equality. The testfails on
master(sameValueError: Unknown type for value ...) andpasses with this change.
Verification (Python 3.12, nixio 1.5.4, numpy 2.5.1):
Scope / caveats for reviewers
datetime64(the literal issue title) is intentionally out ofscope. neo core rejects
numpy.datetime64before it can reach any IO(
ALLOWED_ANNOTATION_TYPESinneo/core/baseneo.pydoes not includenp.datetime64), so the values that actually reach_write_propertyarepython
datetimeobjects held in an object-dtype array — which is what theissue's own diagnosis ("convert datetime objects before saving") refers to.
Broadening core validation to accept
datetime64would be a separate, largerchange.
isinstance(values, list). A length-1 datetime array_annotation is stored andread back as a scalar, then normalised to a 1-element object array by
array_annotate— verified to roundtrip correctly.nixrawio/lazy read path — array_annotations roundtrip is afull-
NixIOfeature.Prepared with AI assistance (Claude Code): the reproduction, fix, and
regression test were AI-drafted, and the reasoning and verification are
documented in full above. Happy to adjust the approach or scope.