Skip to content

Specification for datetime dtypes is ambiguous #85

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

Closed
shoyer opened this issue Oct 16, 2016 · 2 comments · Fixed by #185
Closed

Specification for datetime dtypes is ambiguous #85

shoyer opened this issue Oct 16, 2016 · 2 comments · Fixed by #185
Labels
documentation Improvements to the documentation release notes done Automatically applied to PRs which have release notes.
Milestone

Comments

@shoyer
Copy link
Contributor

shoyer commented Oct 16, 2016

The zarr docs state that datetime and timedelta types are supported, but doesn't specify a fixed time resolution. Without that, the character code is ambiguous.

We should fix this either by indicating a single, fixed time resolution or removing date/time types altogether.

Unfortunately, different systems default to different time resolutions (e.g., us for numpy vs ns for pandas). Given this ambiguity, I would opt to remove date/time types from the spec, and let zarr focus solely on physical rather than logical dtypes.

@alimanfoo
Copy link
Member

Thanks @shoyer, I have no experience of using either dtype so very happy to defer to your judgement here.

@alimanfoo alimanfoo modified the milestone: v2.2 Dec 15, 2016
@alimanfoo alimanfoo mentioned this issue Apr 6, 2017
@alimanfoo
Copy link
Member

Just looking at this, datetime64 is kind-of supported at the moment but it's a mess. There is an issue (https://github.com/alimanfoo/numcodecs/issues/39) that prevents buffer access to a datetime64 array, which can be worked around with a filter, e.g.:

In [34]: a = np.array(['2007-07-13', '2006-01-13', '2010-08-13'], dtype='datetime64[D]')

In [35]: z = zarr.array(a, fill_value=None, filters=[numcodecs.AsType('i8', a.dtype)])

In [36]: z[:]
Out[36]: array(['2007-07-13', '2006-01-13', '2010-08-13'], dtype='datetime64[D]')

Also currently requires fill_value=None otherwise get a JSON serialization exception.

I think I will remove these from the spec for now, and also raise a ValueError if some tries to create an array with datetime or timedelta dtype explaining the situation.

@alimanfoo alimanfoo added documentation Improvements to the documentation release notes done Automatically applied to PRs which have release notes. labels Nov 20, 2017
@alimanfoo alimanfoo removed the to do label Nov 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements to the documentation release notes done Automatically applied to PRs which have release notes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants