Fix datetime encoding precision loss regression for units requiring floating point values #8272
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
spencerkclark
force-pushed
the
fix-8271
branch
from
b11d29b
to
efd95a6
Compare
spencerkclark
force-pushed
the
fix-8271
branch
from
efd95a6
to
8f271a3
Compare
|
@spencerkclark Oh wow, yes, the tests didn't catch this. Your fix makes perfect sense. |
kmuehlbauer
approved these changes
Oct 6, 2023
abkfenris
added a commit
to abkfenris/xpublish
that referenced
this pull request
Oct 12, 2023
v2023.09.0 of Xarray caused our Zarr roundtrip tests to fail as times no longer matched. For full details see xpublish-community#237 The fix looks to have landed in pydata/xarray#8272 and tests against the latest commit works, so we'll go with setting an pin to skip just v2023.09.0. Closes xpublish-community#237
dcherian
added a commit
to dcherian/xarray
that referenced
this pull request
Oct 14, 2023
* upstream/main: (46 commits) xfail flaky test (pydata#8299) Most of mypy 1.6.0 passing (pydata#8296) Rename `reset_encoding` to `drop_encoding` (pydata#8287) Enable `.rolling_exp` to work on dask arrays (pydata#8284) Fix `GroupBy` import (pydata#8286) Ask bug reporters to confirm they're using a recent version of xarray (pydata#8283) Add pyright type checker (pydata#8279) Improved typing of align & broadcast (pydata#8234) Update ci-additional.yaml (pydata#8280) Fix time encoding regression (pydata#8272) Allow a function in `.sortby` method (pydata#8273) make more args kw only (except 'dim') (pydata#6403) Use duck array ops in more places (pydata#8267) Don't raise rename warning if it is a no operation (pydata#8266) Mandate kwargs on `to_zarr` (pydata#8257) copy the `dtypes` module to the `namedarray` package. (pydata#8250) Add xarray-regrid to ecosystem.rst (pydata#8270) Use strict type hinting for namedarray (pydata#8241) Update type annotation for center argument of dataaray_plot methods (pydata#8261) [pre-commit.ci] pre-commit autoupdate (pydata#8262) ...
abkfenris
added a commit
to xpublish-community/xpublish
that referenced
this pull request
Oct 14, 2023
* Anti-pin the latest version of Xarray to v2023.09.0 of Xarray caused our Zarr roundtrip tests to fail as times no longer matched. For full details see #237 The fix looks to have landed in pydata/xarray#8272 and tests against the latest commit works, so we'll go with setting an pin to skip just v2023.09.0. Closes #237 * Dev requirements needs to mirror requirements due to --no-deps
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR proposes a fix to #8271. I think the basic issue is that the only time we need to update the
needed_unitsis if thedata_deltadoes not evenly divide theref_delta. If it does evenly divide it--as it does in the example in #8271--and we try to update theneeded_unitssolely according to the value of theref_delta, we run the risk of resetting them to something that would be coarser than the data requires. If it does not evenly divide it, we are safe to reset theneeded_unitsbecause they will be guaranteed to be finer-grained than the data requires.I modified
test_roundtrip_float_timesto reflect the example given by @larsbuntemeyer in #8271. @kmuehlbauer let me know if this fix makes sense to you.days since#8271whats-new.rst