Skip to content

Use conda-forge netcdftime wherever netcdf4 was tested #1933

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

Merged
merged 7 commits into from
Mar 9, 2018
Merged

Use conda-forge netcdftime wherever netcdf4 was tested #1933

merged 7 commits into from
Mar 9, 2018

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented Feb 21, 2018

@fmaussion
Copy link
Member

Is it safe to do so? I.e. can it hide bugs in the current netCDF4 time handling?

Also I think after #1920 it would be good to have a bit more info in the documentation about why people should switch to netcdftime or not.

@jhamman
Copy link
Member Author

jhamman commented Feb 22, 2018

@fmaussion -

Is it safe to do so? I.e. can it hide bugs in the current netCDF4 time handling?

Save the one test failure here, I think this is going to be a fairly safe change. The netcdftime module in netCDF4 is being ported to a stand-alone package (see: #1048, #1920, Unidata/netcdf4-python#756, and #Unidata/cftime#20). The same test suite is being run on the netcdftime package and we've been working through a series of integration tests with xarray (mostly painless, i.e. #1929).

Also I think after #1920 it would be good to have a bit more info in the documentation about why people should switch to netcdftime or not.

Agreed. Do you think I should include that here?

@fmaussion
Copy link
Member

Agreed. Do you think I should include that here?

As you wish! It can wait until the netcdftime documentation is up and running (most important I guess)

@jhamman
Copy link
Member Author

jhamman commented Feb 23, 2018

@fmaussion - mind reviewing the updated documentation here?

@@ -25,7 +25,7 @@ For netCDF and IO
- `pynio <https://www.pyngl.ucar.edu/Nio.shtml>`__: for reading GRIB and other
geoscience specific file formats
- `zarr <http://zarr.readthedocs.io/>`__: for chunked, compressed, N-dimensional arrays.
- `netcdftime <https://github.com/Unidata/netcdftime>`__: recommended if you
- `netcdftime <https://unidata.github.io/netcdftime>`__: recommended if you
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add: or dates before year 1678 or after year 2262

Copy link
Member

@fmaussion fmaussion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks! Now I'm excited to try it our on the CESM millennial ensemble files ;)

When decoding/encoding datetimes for non-standard calendars or for dates
before year 1678 or after year 2262, xarray uses the `netcdftime`_ library.
``netcdftime`` was previously packaged with the ``netcdf4-python`` package but
is now diestributed separately. ``netcdftime`` is an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: distributed

@jhamman
Copy link
Member Author

jhamman commented Feb 24, 2018

@shoyer - do you think this is good to merge in its current state?

@shoyer
Copy link
Member

shoyer commented Feb 25, 2018

My understanding is that netcdftime hasn't had an official release yet?

Normally, you need to use some sort of prerelease channel and/or flag to download a package that hasn't had a normal release.

@jhamman
Copy link
Member Author

jhamman commented Feb 26, 2018

right now, conda install -c conda-forge netcdftime will give you:

netcdftime:   1.0.0a2-py36_0      conda-forge

I don't andticipate any code changes before the final release. We're still working out the documentation site and whatnot.

netcdf4 did just merge the removal of netcdftime (Unidata/netcdf4-python#756) so this will need to go in prior to any release of netCDF4.

@shoyer
Copy link
Member

shoyer commented Feb 26, 2018

Presumably netcdf4 will add a dependency on netcdftime so they don't break their API for users?

Anyways, if this already work then yes I'm good with merging this.

@jhamman jhamman merged commit 8c6a284 into pydata:master Mar 9, 2018
@jhamman jhamman deleted the test/travis_netcdftime branch March 9, 2018 19:22
@spencerkclark spencerkclark mentioned this pull request Apr 13, 2018
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants