Address review comments

Author: spencerkclark

xarray/coding/cftimeindex.py

@@ -384,9 +384,15 @@ def _add_delta(self, deltas):
         # pandas.  No longer used as of pandas 0.23.
         return self + deltas
 
-    def to_datetimeindex(self):
+    def to_datetimeindex(self, unsafe=False):
         """If possible, convert this index to a pandas.DatetimeIndex.
 
+        Parameters
+        ----------
+        unsafe : bool
+            Flag to turn off warning when converting from a CFTimeIndex with
+            a non-standard calendar to a DatetimeIndex (default ``False``).
+
         Returns
         -------
         pandas.DatetimeIndex
@@ -419,7 +425,7 @@ def to_datetimeindex(self):
         """  # noqa: E501
         nptimes = cftime_to_nptime(self)
         calendar = infer_calendar_name(self)
-        if calendar not in _STANDARD_CALENDARS:
+        if calendar not in _STANDARD_CALENDARS and not unsafe:
             warnings.warn(
                 'Converting a CFTimeIndex with dates from a non-standard '
                 'calendar, {!r}, to a pandas.DatetimeIndex, which uses dates '

xarray/coding/times.py

@@ -280,6 +280,10 @@ def cftime_to_nptime(times):
     new = np.empty(times.shape, dtype='M8[ns]')
     for i, t in np.ndenumerate(times):
         try:
+            # Use pandas.Timestamp in place of datetime.datetime, because
+            # NumPy casts it safely it np.datetime64[ns] for dates outside
+            # 1678 to 2262 (this is not currently the case for
+            # datetime.datetime).
             dt = pd.Timestamp(t.year, t.month, t.day, t.hour, t.minute,
                               t.second)
         except ValueError as e:

xarray/core/common.py

@@ -686,7 +686,7 @@ def resample(self, freq=None, dim=None, how=None, skipna=None,
                             "was passed %r" % dim)
 
         if isinstance(self.indexes[dim_name], CFTimeIndex):
-            raise TypeError(
+            raise NotImplementedError(
                 'Resample is currently not supported along a dimension '
                 'indexed by a CFTimeIndex.  For certain kinds of downsampling '
                 'it may be possible to work around this by converting your '
@@ -723,7 +723,7 @@ def _resample_immediately(self, freq, dim, how, skipna,
                       FutureWarning, stacklevel=3)
 
         if isinstance(self.indexes[dim], CFTimeIndex):
-            raise TypeError(
+            raise NotImplementedError(
                 'Resample is currently not supported along a dimension '
                 'indexed by a CFTimeIndex.  For certain kinds of downsampling '
                 'it may be possible to work around this by converting your '

xarray/core/options.py

@@ -38,8 +38,16 @@ def _set_file_cache_maxsize(value):
     FILE_CACHE.maxsize = value
 
 
+def _warn_on_setting_enable_cftimeindex(enable_cftimeindex):
+    warnings.warn(
+        'The enable_cftimeindex option is now a no-op '
+        'and will be removed in a future version of xarray.',
+        FutureWarning)
+
+
 _SETTERS = {
     FILE_CACHE_MAXSIZE: _set_file_cache_maxsize,
+    ENABLE_CFTIMEINDEX: _warn_on_setting_enable_cftimeindex
 }
 
 
@@ -52,9 +60,6 @@ class set_options(object):
       Default: ``80``.
     - ``arithmetic_join``: DataArray/Dataset alignment in binary operations.
       Default: ``'inner'``.
-    - ``enable_cftimeindex``: flag to enable using a ``CFTimeIndex``
-      for time indexes with non-standard calendars or dates outside the
-      Timestamp-valid range. Default: ``True``.
     - ``file_cache_maxsize``: maximum number of open files to hold in xarray's
       global least-recently-usage cached. This should be smaller than your
       system's per-process file descriptor limit, e.g., ``ulimit -n`` on Linux.
@@ -85,11 +90,6 @@ class set_options(object):
 
     def __init__(self, **kwargs):
         self.old = OPTIONS.copy()
-        if ENABLE_CFTIMEINDEX in kwargs:
-            warnings.warn(
-                'The enable_cftimeindex option is now a no-op '
-                'and will be removed in a future version of xarray.',
-                FutureWarning)
         for k, v in kwargs.items():
             if k not in OPTIONS:
                 raise ValueError(

xarray/tests/test_cftimeindex.py

@@ -361,7 +361,7 @@ def test_groupby(da):
 
 @pytest.mark.skipif(not has_cftime, reason='cftime not installed')
 def test_resample_error(da):
-    with pytest.raises(TypeError):
+    with pytest.raises(NotImplementedError, match='to_datetimeindex'):
         da.resample(time='Y')
 
 
@@ -749,11 +749,12 @@ def test_parse_array_of_cftime_strings():
 
 @pytest.mark.skipif(not has_cftime, reason='cftime not installed')
 @pytest.mark.parametrize('calendar', _ALL_CALENDARS)
-def test_to_datetimeindex(calendar):
+@pytest.mark.parametrize('unsafe', [False, True])
+def test_to_datetimeindex(calendar, unsafe):
     index = xr.cftime_range('2000', periods=5, calendar=calendar)
     expected = pd.date_range('2000', periods=5)
 
-    if calendar in _NON_STANDARD_CALENDARS:
+    if calendar in _NON_STANDARD_CALENDARS and not unsafe:
         with pytest.warns(RuntimeWarning, match='non-standard'):
             result = index.to_datetimeindex()
     else:

xarray/tests/test_dataarray.py

@@ -2281,7 +2281,7 @@ def test_resample_cftimeindex(self):
                                 calendar='noleap')
         array = DataArray(np.arange(12), [('time', times)])
 
-        with raises_regex(TypeError, 'to_datetimeindex'):
+        with raises_regex(NotImplementedError, 'to_datetimeindex'):
             array.resample(time='6H').mean()
 
     def test_resample_first(self):

xarray/tests/test_options.py

@@ -31,11 +31,9 @@ def test_arithmetic_join():
 def test_enable_cftimeindex():
     with pytest.raises(ValueError):
         xarray.set_options(enable_cftimeindex=None)
-    with xarray.set_options(enable_cftimeindex=True):
-        assert OPTIONS['enable_cftimeindex']
-    with pytest.warns(FutureWarning):
+    with pytest.warns(FutureWarning, match='no-op'):
         with xarray.set_options(enable_cftimeindex=True):
-            pass
+            assert OPTIONS['enable_cftimeindex']
 
 
 def test_file_cache_maxsize():