Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Modify _encode_datetime_with_cftime for compatibility with cftime > 1.4.0 #4871

Merged
merged 5 commits into from
Feb 7, 2021
Merged

Modify _encode_datetime_with_cftime for compatibility with cftime > 1.4.0 #4871

merged 5 commits into from
Feb 7, 2021

Conversation

spencerkclark
Copy link
Member

@spencerkclark spencerkclark commented Feb 6, 2021 •
edited
Loading

@spencerkclark spencerkclark mentioned this pull request Feb 6, 2021
Merged
4 tasks
@mathause
Copy link
Collaborator

mathause commented Feb 7, 2021

LGTM - did you check if that makes it slower? (Probably not as vectorize is also a for loop under to hood).

I think the failures are #4873 and #4865

@spencerkclark
Copy link
Member Author

spencerkclark commented Feb 7, 2021 •
edited
Loading

Thanks @mathause -- indeed the timings are pretty similar.

Before:

In [1]: import xarray as xr

In [2]: times = xr.cftime_range("2000", periods=1000).values

In [3]: units = "days since 2000-01-01"

In [4]: calendar = "gregorian"

In [5]: %%timeit
   ...: xr.coding.times._encode_datetime_with_cftime(times, units, calendar)

15.5 ms ± 170 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

After:

In [1]: import xarray as xr

In [2]: times = xr.cftime_range("2000", periods=1000).values

In [3]: units = "days since 2000-01-01"

In [4]: calendar = "gregorian"

In [5]: %%timeit
   ...: xr.coding.times._encode_datetime_with_cftime(times, units, calendar)

14.9 ms ± 160 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

mathause
mathause approved these changes Feb 7, 2021
View reviewed changes
Copy link
Collaborator

@mathause mathause left a comment

Choose a reason for hiding this comment

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

Nice!

@spencerkclark spencerkclark merged commit 46591d2 into pydata:master Feb 7, 2021
@spencerkclark spencerkclark deleted the fix-encoding branch February 7, 2021 23:12
@spencerkclark spencerkclark mentioned this pull request Feb 8, 2021
Closed
dcherian added a commit to DWesl/xarray that referenced this pull request Feb 11, 2021
@dcherian
Merge remote-tracking branch 'upstream/master' into read_grid_mapping…
b65e579 
…_and_bounds_as_coords

* upstream/master: (51 commits)
  Ensure maximum accuracy when encoding and decoding cftime.datetime values (pydata#4758)
  Fix `bounds_error=True` ignored with 1D interpolation (pydata#4855)
  add a drop_conflicts strategy for merging attrs (pydata#4827)
  update pre-commit hooks (mypy) (pydata#4883)
  ensure warnings cannot become errors in assert_ (pydata#4864)
  update pre-commit hooks (pydata#4874)
  small fixes for the docstrings of swap_dims and integrate (pydata#4867)
  Modify _encode_datetime_with_cftime for compatibility with cftime > 1.4.0 (pydata#4871)
  vélin (pydata#4872)
  don't skip the doctests CI (pydata#4869)
  fix da.pad example for numpy 1.20 (pydata#4865)
  temporarily pin dask (pydata#4873)
  Add units if "unit" is in the attrs. (pydata#4850)
  speed up the repr for big MultiIndex objects (pydata#4846)
  dim -> coord in DataArray.integrate (pydata#3993)
  WIP: backend interface, now it uses subclassing  (pydata#4836)
  weighted: small improvements (pydata#4818)
  Update related-projects.rst (pydata#4844)
  iris update doc url (pydata#4845)
  Faster unstacking (pydata#4746)
  ...
@dcherian dcherian mentioned this pull request Feb 11, 2021
Closed
6 tasks
dcherian added a commit to dcherian/xarray that referenced this pull request Feb 12, 2021
@dcherian
Merge remote-tracking branch 'upstream/master' into fix/4107
8ce7175 
* upstream/master: (24 commits)
  Compatibility with dask 2021.02.0 (pydata#4884)
  Ensure maximum accuracy when encoding and decoding cftime.datetime values (pydata#4758)
  Fix `bounds_error=True` ignored with 1D interpolation (pydata#4855)
  add a drop_conflicts strategy for merging attrs (pydata#4827)
  update pre-commit hooks (mypy) (pydata#4883)
  ensure warnings cannot become errors in assert_ (pydata#4864)
  update pre-commit hooks (pydata#4874)
  small fixes for the docstrings of swap_dims and integrate (pydata#4867)
  Modify _encode_datetime_with_cftime for compatibility with cftime > 1.4.0 (pydata#4871)
  vélin (pydata#4872)
  don't skip the doctests CI (pydata#4869)
  fix da.pad example for numpy 1.20 (pydata#4865)
  temporarily pin dask (pydata#4873)
  Add units if "unit" is in the attrs. (pydata#4850)
  speed up the repr for big MultiIndex objects (pydata#4846)
  dim -> coord in DataArray.integrate (pydata#3993)
  WIP: backend interface, now it uses subclassing  (pydata#4836)
  weighted: small improvements (pydata#4818)
  Update related-projects.rst (pydata#4844)
  iris update doc url (pydata#4845)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mathause mathause mathause approved these changes

@dcherian dcherian dcherian approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Time encoding error associated with cftime > 1.4.0
3 participants
@spencerkclark @mathause @dcherian