Fix dayofweek and dayofyear attributes from dates generated by cftime_range (#2633)

* Add workaround for cftime issue 106

* Raise ImportError for too old a version of cftime

* lint

* Simplify version check logic

* Fix test skipping logic

Author: spencerkclark

doc/whats-new.rst

@@ -55,7 +55,8 @@ Enhancements
   reprojection, see  (:issue:`2588`).
   By `Scott Henderson <https://github.com/scottyhq>`_.
 - Like :py:class:`pandas.DatetimeIndex`, :py:class:`CFTimeIndex` now supports
-  "dayofyear" and "dayofweek" accessors (:issue:`2597`).  By `Spencer Clark
+  "dayofyear" and "dayofweek" accessors (:issue:`2597`).  Note this requires a
+  version of cftime greater than 1.0.2.  By `Spencer Clark
   <https://github.com/spencerkclark>`_.
 - The option ``'warn_for_unclosed_files'`` (False by default) has been added to
   allow users to enable a warning when files opened by xarray are deallocated

xarray/coding/cftime_offsets.py

@@ -204,7 +204,11 @@ def _shift_months(date, months, day_option='start'):
         day = _days_in_month(reference)
     else:
         raise ValueError(day_option)
-    return date.replace(year=year, month=month, day=day)
+    # dayofwk=-1 is required to update the dayofwk and dayofyr attributes of
+    # the returned date object in versions of cftime between 1.0.2 and
+    # 1.0.3.4.  It can be removed for versions of cftime greater than
+    # 1.0.3.4.
+    return date.replace(year=year, month=month, day=day, dayofwk=-1)
 
 
 class MonthBegin(BaseCFTimeOffset):

xarray/coding/cftimeindex.py

@@ -44,6 +44,7 @@
 import re
 import warnings
 from datetime import timedelta
+from distutils.version import LooseVersion
 
 import numpy as np
 import pandas as pd
@@ -108,6 +109,11 @@ def _parse_iso8601_with_reso(date_type, timestr):
             replace[attr] = int(value)
             resolution = attr
 
+    # dayofwk=-1 is required to update the dayofwk and dayofyr attributes of
+    # the returned date object in versions of cftime between 1.0.2 and
+    # 1.0.3.4.  It can be removed for versions of cftime greater than
+    # 1.0.3.4.
+    replace['dayofwk'] = -1
     return default.replace(**replace), resolution
 
 
@@ -150,11 +156,21 @@ def get_date_field(datetimes, field):
     return np.array([getattr(date, field) for date in datetimes])
 
 
-def _field_accessor(name, docstring=None):
+def _field_accessor(name, docstring=None, min_cftime_version='0.0'):
     """Adapted from pandas.tseries.index._field_accessor"""
 
-    def f(self):
-        return get_date_field(self._data, name)
+    def f(self, min_cftime_version=min_cftime_version):
+        import cftime
+
+        version = cftime.__version__
+
+        if LooseVersion(version) >= LooseVersion(min_cftime_version):
+            return get_date_field(self._data, name)
+        else:
+            raise ImportError('The {!r} accessor requires a minimum '
+                              'version of cftime of {}. Found an '
+                              'installed version of {}.'.format(
+                                  name, min_cftime_version, version))
 
     f.__name__ = name
     f.__doc__ = docstring
@@ -209,8 +225,10 @@ class CFTimeIndex(pd.Index):
     microsecond = _field_accessor('microsecond',
                                   'The microseconds of the datetime')
     dayofyear = _field_accessor('dayofyr',
-                                'The ordinal day of year of the datetime')
-    dayofweek = _field_accessor('dayofwk', 'The day of week of the datetime')
+                                'The ordinal day of year of the datetime',
+                                '1.0.2.1')
+    dayofweek = _field_accessor('dayofwk', 'The day of week of the datetime',
+                                '1.0.2.1')
     date_type = property(get_date_type)
 
     def __new__(cls, data, name=None):

xarray/tests/__init__.py

@@ -71,6 +71,8 @@ def LooseVersion(vstring):
 has_pynio, requires_pynio = _importorskip('Nio')
 has_pseudonetcdf, requires_pseudonetcdf = _importorskip('PseudoNetCDF')
 has_cftime, requires_cftime = _importorskip('cftime')
+has_cftime_1_0_2_1, requires_cftime_1_0_2_1 = _importorskip(
+    'cftime', minversion='1.0.2.1')
 has_dask, requires_dask = _importorskip('dask')
 has_bottleneck, requires_bottleneck = _importorskip('bottleneck')
 has_rasterio, requires_rasterio = _importorskip('rasterio')

xarray/tests/test_accessors.py

@@ -161,6 +161,8 @@ def times_3d(times):
 @pytest.mark.parametrize('field', ['year', 'month', 'day', 'hour',
                                    'dayofyear', 'dayofweek'])
 def test_field_access(data, field):
+    if field == 'dayofyear' or field == 'dayofweek':
+        pytest.importorskip('cftime', minversion='1.0.2.1')
     result = getattr(data.time.dt, field)
     expected = xr.DataArray(
         getattr(xr.coding.cftimeindex.CFTimeIndex(data.time.values), field),
@@ -176,6 +178,8 @@ def test_field_access(data, field):
 def test_dask_field_access_1d(data, field):
     import dask.array as da
 
+    if field == 'dayofyear' or field == 'dayofweek':
+        pytest.importorskip('cftime', minversion='1.0.2.1')
     expected = xr.DataArray(
         getattr(xr.coding.cftimeindex.CFTimeIndex(data.time.values), field),
         name=field, dims=['time'])
@@ -193,6 +197,8 @@ def test_dask_field_access_1d(data, field):
 def test_dask_field_access(times_3d, data, field):
     import dask.array as da
 
+    if field == 'dayofyear' or field == 'dayofweek':
+        pytest.importorskip('cftime', minversion='1.0.2.1')
     expected = xr.DataArray(
         getattr(xr.coding.cftimeindex.CFTimeIndex(times_3d.values.ravel()),
                 field).reshape(times_3d.shape),

xarray/tests/test_cftime_offsets.py

@@ -1,6 +1,7 @@
 from itertools import product
 
 import numpy as np
+import pandas as pd
 import pytest
 
 from xarray import CFTimeIndex
@@ -797,3 +798,19 @@ def test_calendar_year_length(
     result = cftime_range(start, end, freq='D', closed='left',
                           calendar=calendar)
     assert len(result) == expected_number_of_days
+
+
+@pytest.mark.parametrize('freq', ['A', 'M', 'D'])
+def test_dayofweek_after_cftime_range(freq):
+    pytest.importorskip('cftime', minversion='1.0.2.1')
+    result = cftime_range('2000-02-01', periods=3, freq=freq).dayofweek
+    expected = pd.date_range('2000-02-01', periods=3, freq=freq).dayofweek
+    np.testing.assert_array_equal(result, expected)
+
+
+@pytest.mark.parametrize('freq', ['A', 'M', 'D'])
+def test_dayofyear_after_cftime_range(freq):
+    pytest.importorskip('cftime', minversion='1.0.2.1')
+    result = cftime_range('2000-02-01', periods=3, freq=freq).dayofyear
+    expected = pd.date_range('2000-02-01', periods=3, freq=freq).dayofyear
+    np.testing.assert_array_equal(result, expected)

xarray/tests/test_cftimeindex.py

@@ -12,7 +12,8 @@
     _parsed_string_to_bounds, assert_all_valid_date_type, parse_iso8601)
 from xarray.tests import assert_array_equal, assert_identical
 
-from . import has_cftime, has_cftime_or_netCDF4, raises_regex, requires_cftime
+from . import (has_cftime, has_cftime_1_0_2_1, has_cftime_or_netCDF4,
+               raises_regex, requires_cftime)
 from .test_coding_times import (
     _ALL_CALENDARS, _NON_STANDARD_CALENDARS, _all_cftime_date_types)
 
@@ -175,14 +176,16 @@ def test_cftimeindex_field_accessors(index, field, expected):
     assert_array_equal(result, expected)
 
 
-@pytest.mark.skipif(not has_cftime, reason='cftime not installed')
+@pytest.mark.skipif(not has_cftime_1_0_2_1,
+                    reason='cftime not installed')
 def test_cftimeindex_dayofyear_accessor(index):
     result = index.dayofyear
     expected = [date.dayofyr for date in index]
     assert_array_equal(result, expected)
 
 
-@pytest.mark.skipif(not has_cftime, reason='cftime not installed')
+@pytest.mark.skipif(not has_cftime_1_0_2_1,
+                    reason='cftime not installed')
 def test_cftimeindex_dayofweek_accessor(index):
     result = index.dayofweek
     expected = [date.dayofwk for date in index]