Skip to content

Expose use_cftime option in open_zarr #2886

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
spencerkclark opened this issue Apr 11, 2019 · 7 comments · Fixed by #3229
Closed

Expose use_cftime option in open_zarr #2886

spencerkclark opened this issue Apr 11, 2019 · 7 comments · Fixed by #3229

Comments

@spencerkclark
Copy link
Member

use_cftime was recently added as an option to decode_cf and open_dataset to give users a little more control over how times are decoded (#2759). It would be good if it was also available for open_zarr. This perhaps doesn't have quite the importance, because open_zarr only works for single data stores, so there is no risk of decoding times to different types (e.g. as there was for open_mfdataset, #1263); however, it would still be nice to be able to silence serialization warnings that result from decoding times to cftime objects in some instances, e.g. #2754.

@Geektrovert
Copy link
Contributor

Hey, can I work on this issue? I'm still new to this so a little heads up would be very nice!

@spencerkclark
Copy link
Member Author

Absolutely - I think this should be as simple as adding a use_cftime argument to open_zarr, threading its value through as an argument to the call to conventions.decode_cf here:

ds = conventions.decode_cf(
store, mask_and_scale=mask_and_scale, decode_times=decode_times,
concat_characters=concat_characters, decode_coords=decode_coords,
drop_variables=drop_variables)

and adding a test to make sure it works.

For more general tips on contributing to xarray, see the contributing guide here.

@Geektrovert
Copy link
Contributor

Couldn't figure out how to implement the tests

@naomi-henderson
Copy link

@Geektrovert and @spencerkclark - any progress on this? It would be great to have it working in time for the CMIP6 hackathon in order to suppress all of the warning messages when opening zarr stores. For example, a zarr store using cftime.DatetimeGregorian (e.g., cftime.DatetimeGregorian(2349, 12, 16, 12, 0, 0, 0, 4, 350)) will always generate the warning:

/usr/local/python/anaconda3/envs/pangeoJun2019/lib/python3.6/site-packages/xarray/coding/times.py:465: SerializationWarning: Unable to decode time axis into full numpy.datetime64 objects, continuing using cftime.datetime objects instead, reason: dates out of range
  dtype = _decode_cf_datetime_dtype(data, units, calendar, self.use_cftime)
/usr/local/python/anaconda3/envs/pangeoJun2019/lib/python3.6/site-packages/xarray/coding/times.py:465: SerializationWarning: Unable to decode time axis into full numpy.datetime64 objects, continuing using cftime.datetime objects instead, reason: dates out of range
  dtype = _decode_cf_datetime_dtype(data, units, calendar, self.use_cftime)
/usr/local/python/anaconda3/envs/pangeoJun2019/lib/python3.6/site-packages/numpy/core/numeric.py:538: SerializationWarning: Unable to decode time axis into full numpy.datetime64 objects, continuing using cftime.datetime objects instead, reason: dates out of range
  return array(a, dtype, copy=False, order=order)

For the CMIP6 collection, I always use use_cftime=True in open_mfdataset, but then when I make the zarr store and read it back in, it generates these warnings - always a distraction for anyone not used to dealing with this data

@Geektrovert
Copy link
Contributor

@naomi-henderson, I was working on this. But lately, I have been quite busy so I haven't implemented the tests yet. You can fix it if you wish to.

@rabernat
Copy link
Contributor

Adding tests to #3229 should be pretty straightforward. Great project for someone looking to learn a bit more about xarray internals.

@dcherian dcherian pinned this issue Oct 25, 2019
@max-sixty max-sixty unpinned this issue Mar 4, 2020
@max-sixty
Copy link
Collaborator

@spencerkclark @dcherian for transparency I unpinned this to make room for the broken docs -> github link. Feel free to unpin the build warnings if this is still top-of-mind

andersy005 pushed a commit to Geektrovert/xarray that referenced this issue Aug 25, 2020
dcherian pushed a commit that referenced this issue Sep 2, 2020
* Expose use_cftime option in open_zarr #2886

* Add test for open_zarr w/ use_cftime

* Formatting only

* Add entry in `whats-new.rst`

* Remove space

Co-authored-by: Anderson Banihirwe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants