cftime_range fails for base cftime.datetime object

[aidanheerdegen]

What happened:

xarray.cftime_range does not accept dates that use base classcftime.datetime objects.

What you expected to happen:

I expected xarray.cftime_range to raise an exception that this is an unsupported cftime.datetime type and for the documentation to reflect this.

Minimal Complete Verifiable Example:

import cftime
import xarray
date = cftime.datetime(10,1,1)
xarray.cftime_range(date, periods=3, freq='Y')

Anything else we need to know?:

Returns this error:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-29-d090ea15e436> in <module>
      2 import xarray
      3 date = cftime.datetime(10,1,1)
----> 4 xarray.cftime_range(date, periods=3, freq='Y')

/g/data3/hh5/public/apps/miniconda3/envs/analysis3-20.07/lib/python3.7/site-packages/xarray/coding/cftime_offsets.py in cftime_range(start, end, periods, freq, normalize, name, closed, calendar)
    973     else:
    974         offset = to_offset(freq)
--> 975         dates = np.array(list(_generate_range(start, end, periods, offset)))
    976 
    977     left_closed = False

/g/data3/hh5/public/apps/miniconda3/envs/analysis3-20.07/lib/python3.7/site-packages/xarray/coding/cftime_offsets.py in _generate_range(start, end, periods, offset)
    744     """
    745     if start:
--> 746         start = offset.rollforward(start)
    747 
    748     if end:

/g/data3/hh5/public/apps/miniconda3/envs/analysis3-20.07/lib/python3.7/site-packages/xarray/coding/cftime_offsets.py in rollforward(self, date)
    526     def rollforward(self, date):
    527         """Roll date forward to nearest end of year"""
--> 528         if self.onOffset(date):
    529             return date
    530         else:

/g/data3/hh5/public/apps/miniconda3/envs/analysis3-20.07/lib/python3.7/site-packages/xarray/coding/cftime_offsets.py in onOffset(self, date)
    522         """Check if the given date is in the set of possible dates created
    523         using a length-one version of this offset class."""
--> 524         return date.day == _days_in_month(date) and date.month == self.month
    525 
    526     def rollforward(self, date):

/g/data3/hh5/public/apps/miniconda3/envs/analysis3-20.07/lib/python3.7/site-packages/xarray/coding/cftime_offsets.py in _days_in_month(date)
    195     else:
    196         reference = type(date)(date.year, date.month + 1, 1)
--> 197     return (reference - timedelta(days=1)).day
    198 
    199 

TypeError: unsupported operand type(s) for -: 'cftime._cftime.datetime' and 'datetime.timedelta'

Works if a datetime object with a calendar is used:

import cftime
import xarray
date = cftime.DatetimeGregorian(10,1,1)
xarray.cftime_range(date, periods=3, freq='Y')

Returns:

CFTimeIndex([0010-12-31 00:00:00, 0011-12-31 00:00:00, 0012-12-31 00:00:00], dtype='object')

as expected.

The error occurs here

https://github.com/pydata/xarray/blob/master/xarray/coding/cftime_offsets.py#L197

because this operation is not defined for the base class

https://github.com/Unidata/cftime/blob/master/cftime/_cftime.pyx#L1054

The relevant tests all seem to use datetime strings which are by default standard calendar:

https://github.com/pydata/xarray/blob/master/xarray/coding/cftime_offsets.py#L788

Environment:

Output of xr.show_versions()

INSTALLED VERSIONS

commit: None
python: 3.7.8 | packaged by conda-forge | (default, Jul 31 2020, 02:25:08)
[GCC 7.5.0]
python-bits: 64
OS: Linux
OS-release: 2.6.32-754.18.2.el6.x86_64
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: en_AU.utf8
LANG: en_AU.UTF-8
LOCALE: en_AU.UTF-8
libhdf5: 1.10.5
libnetcdf: 4.7.4

xarray: 0.16.0
pandas: 1.1.0
numpy: 1.19.1
scipy: 1.5.2
netCDF4: 1.5.3
pydap: installed
h5netcdf: 0.8.1
h5py: 2.10.0
Nio: 1.5.5
zarr: 2.4.0
cftime: 1.0.3.4
nc_time_axis: 1.2.0
PseudoNetCDF: None
rasterio: 1.1.5
cfgrib: 0.9.8.4
iris: 2.4.0
bottleneck: 1.3.2
dask: 2.22.0
distributed: 2.22.0
matplotlib: 3.3.0
cartopy: 0.18.0
seaborn: 0.10.1
numbagg: None
pint: 0.14
setuptools: 49.2.0.post20200712
pip: 20.1.1
conda: installed
pytest: 6.0.1
IPython: 7.17.0
sphinx: 3.2.0

[spencerkclark]

Indeed calendar-agnostic cftime.datetime objects are not supported. I agree error messages and documentation could be improved regarding this. We often use cftime.datetime as a shorthand to indicate that any one of cftime.DatetimeNoLeap, cftime.DatetimeGregorian, cftime.DatetimeProlepticGregorian, cftime.DatetimeJulian, cftime.Datetime360Day, cftime.DatetimeAllLeap (all subclasses of cftime.datetime) could be passed as inputs, even though cftime.datetime itself is not supported.

The primary cftime_range tests can be found here, where we do test calendar-aware datetime inputs of all calendar types.

[aidanheerdegen]

Thanks for the link to the tests. Your pytest-fu is strong! You're right, I didn't spot those.

I guess my philosophical point was that this throws an error:

import cftime 
import xarray 
date = cftime.datetime(10,1,1) 
xarray.cftime_range(date, periods=3, freq='Y')  

but this doesn't:

import cftime 
import xarray 
date = cftime.datetime(10,1,1).isoformat() 
xarray.cftime_range(date, periods=3, freq='Y')   

due to the latter being transformed to cftime.DatetimeGregorian as there is a default calendar attribute for cftime_range:

xarray/xarray/coding/cftime_offsets.py

Line 788 in cafab46

calendar="standard",

[spencerkclark]

Ah, I think I see now. So you are arguing that in the case that generic cftime.datetime objects are provided we should cast them to whatever calendar is specified in the calendar argument (in the case that no calendar argument is provided, cftime.DatetimeGregorian)?

[stale]

In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically

[yt87]

What is really confusing:

import cftime
import xarray as xr

first_ = cftime.datetime(2020, 2, 1, calendar="noleap")
last_ = cftime.datetime(2020, 3, 1, calendar="noleap")
first = cftime.DatetimeNoLeap(2020, 2, 1)
last = cftime.DatetimeNoLeap(2020, 3, 1)
assert first_ == first
assert last_ == last

xr.cftime_range(start=first, end=last)
xr.cftime_range(start=first_, end=last_)

CFTimeIndex([2020-02-01 00:00:00, 2020-02-02 00:00:00, 2020-02-03 00:00:00,
             2020-02-04 00:00:00, 2020-02-05 00:00:00, 2020-02-06 00:00:00,
             2020-02-07 00:00:00, 2020-02-08 00:00:00, 2020-02-09 00:00:00,
             2020-02-10 00:00:00, 2020-02-11 00:00:00, 2020-02-12 00:00:00,
             2020-02-13 00:00:00, 2020-02-14 00:00:00, 2020-02-15 00:00:00,
             2020-02-16 00:00:00, 2020-02-17 00:00:00, 2020-02-18 00:00:00,
             2020-02-19 00:00:00, 2020-02-20 00:00:00, 2020-02-21 00:00:00,
             2020-02-22 00:00:00, 2020-02-23 00:00:00, 2020-02-24 00:00:00,
             2020-02-25 00:00:00, 2020-02-26 00:00:00, 2020-02-27 00:00:00,
             2020-02-28 00:00:00, 2020-03-01 00:00:00],
            dtype='object', length=29, calendar='noleap', freq='D')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
File ~/mambaforge/envs/icec/lib/python3.11/site-packages/IPython/core/formatters.py:708, in PlainTextFormatter.__call__(self, obj)
    701 stream = StringIO()
    702 printer = pretty.RepresentationPrinter(stream, self.verbose,
    703     self.max_width, self.newline,
    704     max_seq_length=self.max_seq_length,
    705     singleton_pprinters=self.singleton_printers,
    706     type_pprinters=self.type_printers,
    707     deferred_pprinters=self.deferred_printers)
--> 708 printer.pretty(obj)
    709 printer.flush()
    710 return stream.getvalue()

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/IPython/lib/pretty.py:410, in RepresentationPrinter.pretty(self, obj)
    407                         return meth(obj, self, cycle)
    408                 if cls is not object \
    409                         and callable(cls.__dict__.get('__repr__')):
--> 410                     return _repr_pprint(obj, self, cycle)
    412     return _default_pprint(obj, self, cycle)
    413 finally:

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/IPython/lib/pretty.py:778, in _repr_pprint(obj, p, cycle)
    776 """A pprint that just redirects to the normal repr function."""
    777 # Find newlines and replace them with p.break_()
--> 778 output = repr(obj)
    779 lines = output.splitlines()
    780 with p.group():

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/coding/cftimeindex.py:356, in CFTimeIndex.__repr__(self)
    348     end_str = format_times(
    349         self.values[-REPR_ELLIPSIS_SHOW_ITEMS_FRONT_END:],
    350         display_width,
    351         offset=offset,
    352         first_row_offset=offset,
    353     )
    354     datastr = "\n".join([front_str, f"{' '*offset}...", end_str])
--> 356 attrs_str = format_attrs(self)
    357 # oneliner only if smaller than display_width
    358 full_repr_str = f"{klass_name}([{datastr}], {attrs_str})"

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/coding/cftimeindex.py:276, in format_attrs(index, separator)
    270 def format_attrs(index, separator=", "):
    271     """Format attributes of CFTimeIndex for __repr__."""
    272     attrs = {
    273         "dtype": f"'{index.dtype}'",
    274         "length": f"{len(index)}",
    275         "calendar": f"'{index.calendar}'",
--> 276         "freq": f"'{index.freq}'" if len(index) >= 3 else None,
    277     }
    279     attrs_str = [f"{k}={v}" for k, v in attrs.items()]
    280     attrs_str = f"{separator}".join(attrs_str)

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/coding/cftimeindex.py:703, in CFTimeIndex.freq(self)
    700 """The frequency used by the dates in the index."""
    701 from xarray.coding.frequencies import infer_freq
--> 703 return infer_freq(self)

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/coding/frequencies.py:98, in infer_freq(index)
     95         index = CFTimeIndex(index.values)
     97 if isinstance(index, CFTimeIndex):
---> 98     inferer = _CFTimeFrequencyInferer(index)
     99     return inferer.get_freq()
    101 return pd.infer_freq(index)

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/coding/frequencies.py:107, in _CFTimeFrequencyInferer.__init__(self, index)
    105 def __init__(self, index):
    106     self.index = index
--> 107     self.values = index.asi8
    109     if len(index) < 3:
    110         raise ValueError("Need at least 3 dates to infer frequency")

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/coding/cftimeindex.py:684, in CFTimeIndex.asi8(self)
    680 from xarray.core.resample_cftime import exact_cftime_datetime_difference
    682 epoch = self.date_type(1970, 1, 1)
    683 return np.array(
--> 684     [
    685         _total_microseconds(exact_cftime_datetime_difference(epoch, date))
    686         for date in self.values
    687     ],
    688     dtype=np.int64,
    689 )

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/coding/cftimeindex.py:685, in <listcomp>(.0)
    680 from xarray.core.resample_cftime import exact_cftime_datetime_difference
    682 epoch = self.date_type(1970, 1, 1)
    683 return np.array(
    684     [
--> 685         _total_microseconds(exact_cftime_datetime_difference(epoch, date))
    686         for date in self.values
    687     ],
    688     dtype=np.int64,
    689 )

File ~/mambaforge/envs/icec/lib/python3.11/site-packages/xarray/core/resample_cftime.py:490, in exact_cftime_datetime_difference(a, b)
    456 def exact_cftime_datetime_difference(a: CFTimeDatetime, b: CFTimeDatetime):
    457     """Exact computation of b - a
    458 
    459     Assumes:
   (...)
    488     datetime.timedelta
    489     """
--> 490     seconds = b.replace(microsecond=0) - a.replace(microsecond=0)
    491     seconds = int(round(seconds.total_seconds()))
    492     microseconds = b.microsecond - a.microsecond

File src/cftime/_cftime.pyx:1574, in cftime._cftime.datetime.__sub__()

TypeError: cannot compute the time difference between dates with different calendars

I see two issues here:

1. The wrapper cftime.datetime does not do what is expected.
2. The implementation of == for cftime objects is problematic.

BTW, my environment:
cftime: 1.6.3
xarray: 2023.11.0

[spencerkclark]

Thanks @yt87 — indeed the meaning of this issue has somewhat changed, since it has now become possible for cftime.datetime objects to be calendar-aware. Xarray currently only supports the calendar-specific subclasses of cftime.datetime, but we really should update things to support cftime.datetime objects as well.

I am not sure I completely follow your second point about == being problematic for cftime objects. It seems reasonable for cftime.datetime(2020, 2, 1, calendar="noleap") == cftime.DatetimeNoLeap(2020, 2, 1) to be True. I think what is triggering the error here is that this line is creating a datetime instance with a different calendar than "noleap":

xarray/xarray/coding/cftimeindex.py

Line 682 in 633e66a

epoch = self.date_type(1970, 1, 1)

since in the case of cftime.datetime instances, the date_type alone no longer carries with it a specific calendar type (i.e. this logic needs to be adjusted in xarray).

[yt87]

What I meant was that the arguments in the two invocations of xr.cftime_range are identical, thecalendar attribute for both last and last_ is set to 'noleap', yet the results differ.

xr.cftime_range(start=first, end=last)
xr.cftime_range(start=first_, end=last_)

It appears that the __class__ attribute is what matters, calendar is ignored:

last.__class__
last_.__class__

cftime._cftime.DatetimeNoLeap
cftime._cftime.datetime

In my first reading of the documentation, I assumed that dataclass is a factory method returning classes based on the calendar parameter. Capitalisation confused me.

[spencerkclark]

No worries, I see. To add a little more context, while their type's differ, within the confines of cftime, cftime.datetime(..., calendar="noleap") and cftime.DatetimeNoLeap are meant to do the same thing. This is mainly for backwards compatibility reasons. The goal in cftime is eventually to eliminate the calendar-specific types, in favor of the single calendar-aware type of cftime.datetime. Xarray just has not adapted to this yet (in a few places it still relies on inferring the calendar from the type of the object rather than the calendar attribute, which is not appropriate for objects of type cftime.datetime).