G-Node / python-neo

Fork of python-neo for maintaining the NIX IO.
https://github.com/NeuralEnsemble/python-neo
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Datetimes in annotations/metadata #8

Open achilleas-k opened 7 years ago

achilleas-k commented 7 years ago

Track properties that were converted from datetimes to integers for conversion back to datetime.

achilleas-k commented 7 years ago

Write test for this as well as the rec_datetime and file_datetime attributes.

JuliaSprenger commented 7 years ago

@achilleas-k: For our datasets it would also be nice to have support for datetime.date and datetime.time objects. How complicated would this be to implement?

achilleas-k commented 7 years ago

It doesn't sound like it would be more complicated than what is already mentioned in this issue. I can probably keep a record of what the original format was (datetime, date, or time), in the same way I would do it for simply datetime. Might also be possible to come up with a general solution for types that are supported by Neo (which is basically anything) but not supported in NIX.

I'll have to think about it a bit, but feel free to share suggestions.

gicmo commented 7 years ago

This problem came up numerous times in discussion about nix::Value and also exists for DataArrays (and thus in future maybe DataFrames): We store the (primitive) DataType indicating how the data is stored, but for higher (for the lack of a better word) types, i.e. the interpretation of the data, we currently have no support. The problem is a bit tricky because for e.g. date/time we have three layers: store type: String -> interpretation format (ISO 8601) ->high level type: DateTime. Initially, there was a DataType::DateTime, but it got removed because of this very issue. For opaque times we kinda have the same problem (in DataArrays): storage type: binary blob/octets -> interpretation RGBA (8/8/8/8) -> image. I was working on that a bit, but every time this came up there was a bit of dispute how/where to actually store all this. I was convinced there was a NIX bug open about it, but I cannot find it. Also I do really think this should first be solved at the NIX level, because it is also badly needed there.

gicmo commented 7 years ago

I went ahead and filed issue G-Node/nix#665

achilleas-k commented 7 years ago

Well now you got me thinking whether I should hack around it for Neo at first or whether we should go ahead and put it in NIX. The biggest problem with the latter, implementing higher level data types in NIX, is that it might have to be queued up behind the changes coming in odML. Doing it in Neo can be a good temporary solution for @JuliaSprenger's requirements.

In the worst case, for now I could store two values for each attribute: the raw value and the high-level type (in some unambiguous way). The second could be optional, so we won't need duplicate properties for the primitives.

achilleas-k commented 7 years ago

Future changes to my last suggestion could also be made backwards compatible in the Neo NixIO, especially if the secondary property is optional. During read, when properties are read from a metadata section, for each property X, the IO checks if there is a property X.type and if not, reads in the value as is. If we add support for the type in the future, we just keep the check around in the IO and all is good.

gicmo commented 7 years ago

Does indeed sound like a good plan.