issue_comments

re: using accessors such as get_variables.

To be fair, this pull request didn't really switch the behavior, it was already very similar but with different names (store_variables, open_store_variable) that were now changed to get_variables etc. I chose to continue with getters and setters because it makes it fairly clear what needs to be implemented to make a new DataStore, and allows for some post processing such as _decode_variable_name and encoding/decoding. Its not entirely clear to me how that would fit into a properties based approach so sounds like a good follow up pull request to me.

I personally am not familiar with pandas.Block objects, so I find the name uniformative. That combined with the fact that renaming Variable to Block would break alignment with netCDF naming conventions (so confuse users coming from that background) makes me hesitant about the change. I think I'd be more excited about finding a better name for noncoordinates (fields? ) instead of renaming variables (which would also cause non-backwards compatibility)

At least temporarily you might consider this:

myds = xray.open_dataset(ds.dumps())

One possibility could be to have the encoding filtering only happen once if the variable was loaded from NetCDF4. Ie, if a variable with chunksizes encoding were loaded from file they it would be removed after the first attempt to index, afterwards all encodings persist. I've been experimenting with something along those lines but don't have it working perfectly yet.

Is there a reason why we don't just have it remove problematic encodings? Some encodings are certainly nice to persist (fill value, scale, offset, time units etc ...)

So ... do you want me to remove the tests and only include the .data -> .values fix?

Re: api. I'm working on a better storage scheme for reflectivity data, which involves CF decoding plus remapping some values. I could build my own data store which implements the netcdf store and adds the extra layer there .. but just using decode_cf_variable is far easier. In general, I can imagine a situation where we would want to allow users to provide their own decoding functions, but thats a larger project.

I'm confused, are you saying we shouldn't add tests for internal functions?

Yeah I agree, seems like a great option to have.

Also worth considering: how should datetime64[us] datetimes be handled? Currently they get cast to [ns] which, since datetimes do not, could get confusing.

Yeah all fixed.

In #125 I went the route of forcing datetimes to be datetime64[ns]. This is probably part of a broader conversation, but doing so might save some future headaches. Of course ... it would also restrict us to nanosecond precision. Basically I feel like we should either force datetimes to be datetime64[ns] or make sure that operations on datetime objects preserve their type.

Probably worth getting this in and picking that conversation back up if needed. In which case could you add tests which make sure variables with datetime objects are still datetime objects after concatenation? If those start getting cast to datetime[ns] it'll start get confusing for users.

The reorganization does make things cleaner, but the behavior changed relative to #125. In particular, while this patch fixes concatenation with datetime64 times it doesn't work with datetimes:

In [2]: dates = [datetime.datetime(2011, 1, i + 1) for i in range(10)] In [3]: ds = xray.Dataset({'time': ('time', dates)}) In [4]: xray.Dataset.concat([ds.indexed(time=slice(0, 4)), ds.indexed(time=slice(4, 8))], 'time')['time'].values Out[4]: array([1293840000000000000L, 1293926400000000000L, 1294012800000000000L, 1294099200000000000L, 1294185600000000000L, 1294272000000000000L, 1294358400000000000L, 1294444800000000000L], dtype=object)

and not sure if this was broken before or not ...

In [5]: xray.Dataset.concat([x for _, x in ds.groupby('time')], 'time')['time'].values ValueError: cannot slice a 0-d array

Yeah this gets tricky. Fixed part of the problem by reverting to using np.asarray instead of as_array_or_item in NumpyArrayWrapper. But I'm not sure thats the full solution, like you mentioned the problem is much deeper, though I don't think pushing the datetime nastiness into higher level functions (such as concat) is a great option.

Also, I've been hoping to get the way dates are handled to be slightly more consistent, since as it currently stands its hard to know which data type dates are being stored as.

```

d64 = np.datetime64(d) print xray.Variable(['time'], [d]).dtype dtype('O') print xray.Variable(['time'], [d64]).dtype dtype('<M8[ns]') print d64.dtype dtype('<M8[us]') ```

I'm going to attempt getting utils.as_safe_array to convert from datetime objects to datetime64 objects which should make things a little clearer, but still doesn't solve the whole problem.

@alimanfoo

Glad you're enjoying xray!

From your description it sounds like it should be relatively simple for you to get xray working with your dataset. NetCDF4 is a subset of h5py and simply adding dimension scales should get you most of the way there.

Re: groups, each xray.Dataset corresponds to one HDF5 group. So while xray doesn't currently support groups, you could split your HDF5 dataset into separate files for each group and load those files using xray. Alternatively (if you feel ambitious) it shouldn't be too hard to get xray's NetCDF4DataStore (backends.netCDF4_.py) to work with groups, allowing you to do something like:

dataset = xray.open_dataset('multiple_groups.h5', group='/one_group')

This gives some good examples of how groups work within the netCDF4.

Also, as @shoyer mentioned, it might make sense to modify xray so that NetCDF4 support is obtained by wrapping h5py instead of netCDF4 which might make your life even easier.

Sounds good, I'd also like to consider renaming the behavior/arguments in concat. Or maybe add an additional argument with options along the lines of 'shared-dimension', 'different', 'all'. Which would allow the user to choose which variables are concatenated (ie, variables with dimension == 'concat_dimension' those that are different across datasets (even if they don't follow 'concat_dimension') and all variables.

I like the new look, more intuitive.

One thought, what happens when a variable has a coordinate twice? For example a covariance matrix?

+1 I like the name DataArray much more than DatasetArray

Yeah, does seem that way. Unfortunately that makes comparison with datetime objects a bit awkward. Perhaps we should include a to_datetime() the way pandas does? Certainly, it would be convenient to be able to do comparisons such as:

ds['time'].data > datetime.datetime(2014, 01, 01)

This is definitely not high priority though.

Renaming set_variables to update seems reasonable to me. Its behavior seems similar enough to dict.update. Then a separate function replace could be a lightweight wrapper around update which makes a copy:

obj = type(self)(variables, self.attributes) obj.update(_args, *_kwdargs)

I'll go ahead and merge this in to fix the bug ... but I wonder if there is any way we can avoid using np.datetime64 objects. They seem under-developed / broken.

Yes much clearer, thanks. As soon as build passes I'll merge it in.

@shoyer You're right I can serialize the latitude object directly from that opendap url ... but after some manipulation I run into this:

ipdb> print fcst dimensions: latitude = 31 longitude = 46 time = 7 variables: object latitude(latitude) units:degrees_north _CoordinateAxisType:Lat object longitude(longitude) units:degrees_east _CoordinateAxisType:Lon datet... time(time) standard_name:time _CoordinateAxisType:Time units:hours since 2014-03-03 00:0... ipdb> fcst.dump('./test.nc') *** TypeError: illegal primitive data type, must be one of ['i8', 'f4', 'u8', 'i1', 'U1', 'S1', 'i2', 'u1', 'i4', 'u2', 'f8', 'u4'], got object

Currently tracking down exactly whats going on here.

Yeah I think keeping them transparent to the user except when reading/writing is the way to go. Two datasets with the same data but different encodings should still be equal when compared, and operations beyond slicing should probably destroy encodings. Not sure how to handle the various file formats, like you said it could be all part of the store, or we could just throw warnings/fail if encodings aren't feasible.

Another similar option would be to use in-memory HDF5 objects for which Todd Small found an option:

Writing to a string:

h5_file = tables.open_file("in-memory", title=my_title, mode="w", 12 driver="H5FD_CORE", driver_core_backing_store=0) ... [add variables] ... image = h5_file.get_file_image()

Reading from a string

h5_file = tables.open_file("in-memory", mode="r", driver="H5FD_CORE", driver_core_image=image, driver_core_backing_store=0)

Currently this renames CF time units to 'cf_units' but perhaps '_units' or even just keeping 'units' would be better. Thoughts?

@jreback I'll spend some time getting a better feel for how/if we could push some of the backend into pandas' HDFStore. Certainly, we'd like to leverage other more powerful packages (pandas, numpy) as much as possible. Thanks for the suggestion.

This is all great. I've been experimenting with this branch and the majority of it is running fine. Given that this project is still under heavy development and rather than bloating this pull request, lets go ahead and merge it into master and iterate on top of it.

Sounds good. I haven't had time to make much progress over the last few days, but do have some ideas about next steps and what this project has to offer that iris doesn't. I'll get both formalized next time I have a chance. In the meantime, feedback would be great!