pangeo-data / climpred

:earth_americas: Verification of weather and climate forecasts :earth_africa:
https://climpred.readthedocs.io
MIT License
225 stars 49 forks source link

`sel_time` time selection requires formatting of string for non-normal calendar years #859

Open gmacgilchrist opened 4 weeks ago

gmacgilchrist commented 4 weeks ago

Describe the bug If a year is prior to 1000, such as for the piControl run used for perfect model experiments, sel_time fails when trying to find a time based on the integer of the year. There is a simple fix to implement zfill(4) to format the string from which the time is being selected.

Code Sample

import cftime
import xarray as xr

ds = xr.Dataset(coords={'time':[cftime.DatetimeNoLeap(300,1,1),cftime.DatetimeNoLeap(301,1,1)]})
year_int = ds.time[0].dt.year.values
ds.sel(time=str(year_int))

fails with

ValueError: no ISO-8601 or cftime-string-like match for string: 300

The following change fixes this issue:

import xarray as xr

ds = xr.Dataset(coords={'time':[cftime.DatetimeNoLeap(300,1,1),cftime.DatetimeNoLeap(301,1,1)]})
year_int = ds.time[0].dt.year.values
ds.sel(time=str(year_int).zfill(4))

Happy to implement this PR, but wasn't completely sure if that change is likely to break any other aspects of the code, e.g. for other datasets, and I haven't yet had time to implement the changes to check, so I'm just logging the issue here.

Output of climpred.show_versions()

``` INSTALLED VERSIONS ------------------ commit: None python: 3.11.9 | packaged by conda-forge | (main, Apr 19 2024, 18:36:13) [GCC 12.3.0] python-bits: 64 OS: Linux OS-release: 4.18.0-553.5.1.el8_10.x86_64 machine: x86_64 processor: x86_64 byteorder: little LC_ALL: None LANG: en_US.UTF-8 LOCALE: en_US.UTF-8 climpred: 2.4.0 xarray: 2023.12.0 pandas: 2.2.2 numpy: 1.26.4 scipy: 1.13.1 cftime: 1.6.3 netcdf4: None nc_time_axis: 1.4.1 matplotlib: 3.8.2 cf_xarray: 0.9.2 xclim: 0.50.0 dask: 2024.5.0 distributed: 2024.5.0 setuptools: 69.5.1 pip: 24.0 conda: None IPython: 8.25.0 sphinx: None ```
aaronspring commented 4 weeks ago

Open for a PR. Don't see how this should break anything.

But also a bit unsure why a PR would be needed and what alternative workarounds could be, ie. setting new calendar / time coordinate before using climpred. Do you have a sample code example of what's not possible at the moment?