Update time coding tests to assert exact equality #9961
Conversation
| if np.timedelta64(1, time_unit) > np.timedelta64(1, minimum_resolution): | ||
| expected_unit = minimum_resolution | ||
| else: | ||
| expected_unit = time_unit | ||
| expected = cftime_to_nptime(expected, time_unit=expected_unit) |
There was a problem hiding this comment.
I needed to add this logic specifically for this test:
(12300 + np.arange(5), "hour since 1680-01-01 00:00:00.500000", "us"),
This is because the reference date required the dates be decoded to microsecond resolution (really only millisecond resolution would be required, but we follow pandas's lead here). Otherwise for time_unit="s" we would truncate precision when converting the cftime objects to np.datetime64 values, which prevented asserting exact equality.
| @@ -220,7 +220,7 @@ def test_decode_standard_calendar_inside_timestamp_range( | |||
| ) -> None: | |||
| import cftime | |||
|
|
|||
| units = "days since 0001-01-01" | |||
| units = "hours since 0001-01-01" | |||
There was a problem hiding this comment.
Using encoding units of "days" unnecessarily led the times to be encoded with floats, which prevented asserting exact equality in this test. Testing floating point time decoding was not the point of this test, so I changed to encoding the times with units of "hours" instead.
|
@spencerkclark Adding the plan-to-merge label. Is there anything you want to add here? Otherwise this is good to merge AFAICT. |
|
Thanks for the review @kmuehlbauer—indeed I think this should be ready to go. I will go ahead and merge. |
Per #9618 (comment), this PR updates relevant time coding tests to assert exact equality rather than equality with a tolerance, since xarray's minimum supported version of cftime has been greater than 1.2.1 for a while now.
cc: @kmuehlbauer
whats-new.rst