issue_comments

Up to you, but yes it would be greatly appreciated if you could work on this.

On Mon, Oct 17, 2016 at 12:15 PM, Julia Signell notifications@github.com wrote:

@shoyer https://github.com/shoyer is this something I should be working on? I am happy to, just don't know how this normally goes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/917#issuecomment-254304733, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1qqKU4SU-c_e533xHM0uyBibdRLpks5q08lGgaJpZM4JS9Hw .

We should recommend decode_times=False in the docs but I would still make datetime/timedelta objects -- this feels like something that should be left up to users.

I think we should try to convert numpy scalars/arrays, because otherwise the data won't convert directly to JSON. Possibly could take a duck typing approach of looking for .item() or .tolist() methods?

Thank you @jsignell -- really nice work here!

I have a few minor suggestions, but otherwise this looks very nice!

To ensure that the documentation subpages are built for these methods (which makes the sphinx link work), you need to add them to doc/api.rst. These should go in the IO / Conversion section.

If you haven't tested the docs locally with make html, take a look at the development version of the docs (http://xarray.pydata.org/en/latest/) about 5-10 minutes after I merge this to make sure everything looks right. RST can be tricky to get right.

This looks like a great start, but to be really useful it needs a couple other things: - [x] corresponding Dataset.from_dict and DataArray.from_dict class methods for creating xarray datasets from the output of to_dict - [x] unit tests verifying that these methods output the expected thing, and that a dataset can be faithfully round-tripped through to_dict/from_dict - [x] a section in the "Serialization and IO" docs describing how to use these methods - [x] what's new note

Please ask if you need guidance on where to start for any of these! This will be a very welcome addition to xarray :).

This is looking pretty good! I think the main thing this needs now is a short section in the docs on IO methods :). Also, a note in "What's New".

It would also be good to test roundtripping arrays with some NaN and NaT values as well

One big thing that from_dict needs is validation. If the input dict does not match the expected format (e.g., there is a missing field), the user should get a sensible error message so they understand what went wrong.

I think the datetime issue is a numpy bug related to https://github.com/numpy/numpy/issues/7619

We can work around this by casting to datetime64[ns]:

(Pdb) ds['t'].values.tolist() [1356998400000000000, 1357084800000000000, 1357171200000000000, 1357257600000000000, 1357344000000000000, 1357430400000000000, 1357516800000000000, 1357603200000000000, 1357689600000000000, 1357776000000000000] (Pdb) ds['t'].values.astype('datetime64[us]').tolist() [datetime.datetime(2013, 1, 1, 0, 0), datetime.datetime(2013, 1, 2, 0, 0), datetime.datetime(2013, 1, 3, 0, 0), datetime.datetime(2013, 1, 4, 0, 0), datetime.datetime(2013, 1, 5, 0, 0), datetime.datetime(2013, 1, 6, 0, 0), datetime.datetime(2013, 1, 7, 0, 0), datetime.datetime(2013, 1, 8, 0, 0), datetime.datetime(2013, 1, 9, 0, 0), datetime.datetime(2013, 1, 10, 0, 0)]

The same issue holds for timedelta:

(Pdb) (ds['t'] - ds['t'][0]).values.tolist() [0, 86400000000000, 172800000000000, 259200000000000, 345600000000000, 432000000000000, 518400000000000, 604800000000000, 691200000000000, 777600000000000] (Pdb) (ds['t'] - ds['t'][0]).values.astype('timedelta64[us]').tolist() [datetime.timedelta(0), datetime.timedelta(1), datetime.timedelta(2), datetime.timedelta(3), datetime.timedelta(4), datetime.timedelta(5), datetime.timedelta(6), datetime.timedelta(7), datetime.timedelta(8), datetime.timedelta(9)]

We can work around these by checking for timedelta64 or datetime64 dtypes (use np.issubdtype(values.dtype, np.datetime64), and using astype to convert to datetime64[us]/timedelta64[us].