Support datetime64 in IO module. #45
Conversation
src/scitiff/io.py
Outdated
| """Warning for broken scitiff metadata.""" | ||
|
|
||
|
|
||
| def _from_dict(dict_repr_var: dict) -> sc.Variable: |
There was a problem hiding this comment.
Can you give this a more distinct name? I got confused about _from_dict and from_dict.
| else: | ||
| values = np.datetime64(values) | ||
| corrected_repr_var['values'] = values | ||
| return from_dict(corrected_repr_var) |
There was a problem hiding this comment.
Does this handle units correctly? I don't remember whether the variable constructor converts the datetime to the provided unit.
There was a problem hiding this comment.
In what way? I just tried this:
var = sc.datetime('now', unit='s')
display(var)
new_var = sc.datetime(str(var.value), unit='hour')
display(new_var)and it gave me
datetime64 [s] 2026-01-20T15:21:32 and datetime64 [h] 2026-01-20T15
respectively.
Or did you mean some other cases...?
There was a problem hiding this comment.
I think the general approach is fine. Storing datetime as ISO strings is pretty common and robust. But this seems to me like scipp's from_dict should be able to handle it. Did you consider updating that?
There was a problem hiding this comment.
It's not really about making it to a dictionary though.
It's only when it has to be part of the pydantic Model instance, which will be serialized as a json at the end.
Do you think it'll be helpful that from_dict turns the datetime64 into a string value...?
Co-authored-by: Jan-Lukas Wynen <j-l.wynen@hotmail.de>
… the original one. [skip ci]
ScitiffMetadat schema does not support datetime64 dtype but IO module can handle it with a few if-statements.
It might be very convenient to store/load some important dates and timestamp...?
But it means we customzie the serialize/deserialize function and may be surprising to someone who reads the metadata in a different way...
Or is there a better way...?