Don't crash parsing ABF files with an invalid acquisition date#1866
Don't crash parsing ABF files with an invalid acquisition date#1866h-mayorquin wants to merge 1 commit into
Conversation
zm711
left a comment
There was a problem hiding this comment.
Just one general question @h-mayorquin.
| try: | ||
| header["rec_datetime"] = datetime.datetime(YY, MM, DD, hh, mm, ss, ms) | ||
| except (ValueError, OverflowError): | ||
| # Date/time header fields hold an out-of-range or "no date" sentinel |
There was a problem hiding this comment.
Should we warn people? I don't know this system well enough to know if warning a person that they didn't have an acquisition time is a marker of a "poor" acquisition or is something that the user should know they forgot to set up? What do you think? It's not actionable, but it might be valuable for the user to know. I just don't know the reader.
There was a problem hiding this comment.
I am honestly not sure a missing date is a real problem, so I would rather follow pyABF precedent here to just skip it. As far as I can tell it happens because the date field holds an unset/sentinel value (0xFFFFFFFF) that does not parse into a real date, and pyABF does not treat that as fatal either, it just falls back.
In addition, most users of neo do not really use this field so I don't think the warning will be useful for their purposes. Most of the datetime work was done to parse metadata for neuroconv where this is required and there it fails loudly so I think is Ok.
Btw, thanks for making me check this, I found another error that I will address.
Some ABF files store a "no date" sentinel (all bits set) in their date header fields, and
AxonRawIOcurrently raises while building the recording datetime from those values, which blocks reading the file entirely even though the date is non-essential annotation. Both the ABF v1 and ABF v2 paths produce out-of-range values from this sentinel, so parsing now falls back to aNonerecording datetime rather than failing, keeping the signal accessible.