Repository: pandas-dev/pandas
Pull request: standardize signature for Index reductions, implement nanmean for datetime64 dtypes
Same basic goal as #23890, but focusing on nanops (a module I'm not all that familiar with) first, with the array methods later, instead of vice-versa.

I think the only user-facing change (i.e. need for whatsnew) is that `Series[datetime64].mean()` now works.

I need to see if there are any more tests from #23890 or #24024 that can be copied here.  

- [x] closes #24265
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Problem statement:
Series.max incorrect for skipna=False with missing datetime64[ns] data.
```python
In [2]: s = pd.Series([1, None, 3], dtype='datetime64[ns]')

In [3]: s.max(skipna=False)
Out[3]: Timestamp('1970-01-01 00:00:00.000000003')
```

That should be `NaT`.

We get `min` correct, but think just because iNaT is already the min.

Probably affects datetimetz / timedelta as well.

Hints from issue discussion:

Commit message: implement view

Patch to review:
diff --git a/pandas/core/arrays/datetimelike.py b/pandas/core/arrays/datetimelike.py
index fa6941476522d..ffa37738e0a06 100644
--- a/pandas/core/arrays/datetimelike.py
+++ b/pandas/core/arrays/datetimelike.py
@@ -483,6 +483,26 @@ def _maybe_mask_results(self, result, fill_value=iNaT, convert=None):
             result[self._isnan] = fill_value
         return result
 
+    # ------------------------------------------------------------------
+    # Additional array methods
+    # These are not part of the EA API, but we implement them because
+    # pandas currently assumes they're there.
+
+    def view(self, dtype=None):
+        """
+        New view on this array with the same data.
+
+        Parameters
+        ----------
+        dtype : numpy dtype, optional
+
+        Returns
+        -------
+        ndarray
+            With the specified `dtype`.
+        """
+        return self._data.view(dtype=dtype)
+
     # ------------------------------------------------------------------
     # Frequency Properties/Methods
 
diff --git a/pandas/core/base.py b/pandas/core/base.py
index 928e90977f95b..0b7fb29d792af 100644
--- a/pandas/core/base.py
+++ b/pandas/core/base.py
@@ -920,10 +920,16 @@ def _ndarray_values(self):
     def empty(self):
         return not self.size
 
-    def max(self):
+    def max(self, axis=None, skipna=True):
         """
         Return the maximum value of the Index.
 
+        Parameters
+        ----------
+        axis : {None}
+            Dummy argument for consistency with Series
+        skipna : bool, default True
+
         Returns
         -------
         scalar
@@ -951,22 +957,36 @@ def max(self):
         >>> idx.max()
         ('b', 2)
         """
+        nv.validate_minmax_axis(axis)
         return nanops.nanmax(self.values)
 
-    def argmax(self, axis=None):
+    def argmax(self, axis=None, skipna=True):
         """
         Return a ndarray of the maximum argument indexer.
 
+        Parameters
+        ----------
+        axis : {None}
+            Dummy argument for consistency with Series
+        skipna : bool, default True
+
         See Also
         --------
         numpy.ndarray.argmax
         """
+        nv.validate_minmax_axis(axis)
         return nanops.nanargmax(self.values)
 
-    def min(self):
+    def min(self, axis=None, skipna=True):
         """
         Return the minimum value of the Index.
 
+        Parameters
+        ----------
+        axis : {None}
+            Dummy argument for consistency with Series
+        skipna : bool, default True
+
         Returns
         -------
         scalar
@@ -994,16 +1014,24 @@ def min(self):
         >>> idx.min()
         ('a', 1)
         """
+        nv.validate_minmax_axis(axis)
         return nanops.nanmin(self.values)
 
-    def argmin(self, axis=None):
+    def argmin(self, axis=None, skipna=True):
         """
         Return a ndarray of the minimum argument indexer.
 
+        Parameters
+        ----------
+        axis : {None}
+            Dummy argument for consistency with Series
+        skipna : bool, default True
+
         See Also
         --------
         numpy.ndarray.argmin
         """
+        nv.validate_minmax_axis(axis)
         return nanops.nanargmin(self.values)
 
     def tolist(self):
diff --git a/pandas/core/dtypes/missing.py b/pandas/core/dtypes/missing.py
index 809dcbd054ea0..0054a7c65b91a 100644
--- a/pandas/core/dtypes/missing.py
+++ b/pandas/core/dtypes/missing.py
@@ -14,7 +14,8 @@
     is_period_dtype, is_scalar, is_string_dtype, is_string_like_dtype,
     is_timedelta64_dtype, needs_i8_conversion, pandas_dtype)
 from .generic import (
-    ABCExtensionArray, ABCGeneric, ABCIndexClass, ABCMultiIndex, ABCSeries)
+    ABCDatetimeArray, ABCExtensionArray, ABCGeneric, ABCIndexClass,
+    ABCMultiIndex, ABCSeries)
 from .inference import is_list_like
 
 isposinf_scalar = libmissing.isposinf_scalar
@@ -108,7 +109,7 @@ def _isna_new(obj):
     elif isinstance(obj, ABCMultiIndex):
         raise NotImplementedError("isna is not defined for MultiIndex")
     elif isinstance(obj, (ABCSeries, np.ndarray, ABCIndexClass,
-                          ABCExtensionArray)):
+                          ABCExtensionArray, ABCDatetimeArray)):
         return _isna_ndarraylike(obj)
     elif isinstance(obj, ABCGeneric):
         return obj._constructor(obj._data.isna(func=isna))
@@ -196,6 +197,8 @@ def _isna_ndarraylike(obj):
         else:
             values = obj
         result = values.isna()
+    elif isinstance(obj, ABCDatetimeArray):
+        return obj.isna()
     elif is_string_dtype(dtype):
         # Working around NumPy ticket 1542
         shape = values.shape
diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py
index dd2537c11a94c..bd5d261d53f26 100644
--- a/pandas/core/indexes/datetimelike.py
+++ b/pandas/core/indexes/datetimelike.py
@@ -41,6 +41,7 @@ class DatetimeIndexOpsMixin(DatetimeLikeArrayMixin):
     copy = Index.copy
     unique = Index.unique
     take = Index.take
+    view = Index.view
 
     # DatetimeLikeArrayMixin assumes subclasses are mutable, so these are
     # properties there.  They can be made into cache_readonly for Index
@@ -258,7 +259,7 @@ def tolist(self):
         """
         return list(self.astype(object))
 
-    def min(self, axis=None, *args, **kwargs):
+    def min(self, axis=None, skipna=True, *args, **kwargs):
         """
         Return the minimum value of the Index or minimum along
         an axis.
@@ -286,7 +287,7 @@ def min(self, axis=None, *args, **kwargs):
         except ValueError:
             return self._na_value
 
-    def argmin(self, axis=None, *args, **kwargs):
+    def argmin(self, axis=None, skipna=True, *args, **kwargs):
         """
         Returns the indices of the minimum values along an axis.
 
@@ -309,7 +310,7 @@ def argmin(self, axis=None, *args, **kwargs):
             i8[mask] = np.iinfo('int64').max
         return i8.argmin()
 
-    def max(self, axis=None, *args, **kwargs):
+    def max(self, axis=None, skipna=True, *args, **kwargs):
         """
         Return the maximum value of the Index or maximum along
         an axis.
@@ -337,7 +338,7 @@ def max(self, axis=None, *args, **kwargs):
         except ValueError:
             return self._na_value
 
-    def argmax(self, axis=None, *args, **kwargs):
+    def argmax(self, axis=None, skipna=True, *args, **kwargs):
         """
         Returns the indices of the maximum values along an axis.
 
diff --git a/pandas/core/indexes/range.py b/pandas/core/indexes/range.py
index 0da924de244ed..110c9f4025bd8 100644
--- a/pandas/core/indexes/range.py
+++ b/pandas/core/indexes/range.py
@@ -297,12 +297,14 @@ def _minmax(self, meth):
 
         return self._start + self._step * no_steps
 
-    def min(self):
+    def min(self, axis=None, skipna=True):
         """The minimum value of the RangeIndex"""
+        nv.validate_minmax_axis(axis)
         return self._minmax('min')
 
-    def max(self):
+    def max(self, axis=None, skipna=True):
         """The maximum value of the RangeIndex"""
+        nv.validate_minmax_axis(axis)
         return self._minmax('max')
 
     def argsort(self, *args, **kwargs):
diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py
index 027f458614bd8..9e1d3d7b9fd62 100644
--- a/pandas/core/nanops.py
+++ b/pandas/core/nanops.py
@@ -6,14 +6,14 @@
 
 import numpy as np
 
-from pandas._libs import lib, tslibs
+from pandas._libs import iNaT, lib, tslibs
 import pandas.compat as compat
 
 from pandas.core.dtypes.cast import _int64_max, maybe_upcast_putmask
 from pandas.core.dtypes.common import (
     _get_dtype, is_any_int_dtype, is_bool_dtype, is_complex, is_complex_dtype,
-    is_datetime64_dtype, is_datetime_or_timedelta_dtype, is_float,
-    is_float_dtype, is_integer, is_integer_dtype, is_numeric_dtype,
+    is_datetime64_dtype, is_datetime64tz_dtype, is_datetime_or_timedelta_dtype,
+    is_float, is_float_dtype, is_integer, is_integer_dtype, is_numeric_dtype,
     is_object_dtype, is_scalar, is_timedelta64_dtype)
 from pandas.core.dtypes.missing import isna, na_value_for_dtype, notna
 
@@ -203,6 +203,7 @@ def _get_values(values, skipna, fill_value=None, fill_value_typ=None,
     if necessary copy and mask using the specified fill_value
     copy = True will force the copy
     """
+    orig_values = values
     values = com.values_from_object(values)
 
     if mask is None:
@@ -212,6 +213,10 @@ def _get_values(values, skipna, fill_value=None, fill_value_typ=None,
             mask = isna(values)
 
     dtype = values.dtype
+    if is_datetime64tz_dtype(orig_values):
+        dtype = orig_values.dtype
+
+    values = getattr(values, 'asi8', values)
     dtype_ok = _na_ok_dtype(dtype)
 
     # get our fill value (in case we need to provide an alternative
@@ -268,12 +273,16 @@ def _view_if_needed(values):
 def _wrap_results(result, dtype, fill_value=None):
     """ wrap our results if needed """
 
-    if is_datetime64_dtype(dtype):
+    if is_datetime64_dtype(dtype) or is_datetime64tz_dtype(dtype):
+        if fill_value is None:
+            # GH#24293
+            fill_value = iNaT
         if not isinstance(result, np.ndarray):
+            tz = getattr(dtype, 'tz', None)
             assert not isna(fill_value), "Expected non-null fill_value"
             if result == fill_value:
                 result = np.nan
-            result = tslibs.Timestamp(result)
+            result = tslibs.Timestamp(result, tz=tz)
         else:
             result = result.view(dtype)
     elif is_timedelta64_dtype(dtype):
@@ -426,7 +435,6 @@ def nansum(values, axis=None, skipna=True, min_count=0, mask=None):
     return _wrap_results(the_sum, dtype)
 
 
-@disallow('M8')
 @bottleneck_switch()
 def nanmean(values, axis=None, skipna=True, mask=None):
     """
@@ -457,7 +465,8 @@ def nanmean(values, axis=None, skipna=True, mask=None):
         values, skipna, 0, mask=mask)
     dtype_sum = dtype_max
     dtype_count = np.float64
-    if is_integer_dtype(dtype) or is_timedelta64_dtype(dtype):
+    if (is_integer_dtype(dtype) or is_timedelta64_dtype(dtype) or
+            is_datetime64_dtype(dtype) or is_datetime64tz_dtype(dtype)):
         dtype_sum = np.float64
     elif is_float_dtype(dtype):
         dtype_sum = dtype
@@ -466,7 +475,9 @@ def nanmean(values, axis=None, skipna=True, mask=None):
     the_sum = _ensure_numeric(values.sum(axis, dtype=dtype_sum))
 
     if axis is not None and getattr(the_sum, 'ndim', False):
-        the_mean = the_sum / count
+        with np.errstate(all="ignore"):
+            # suppress division by zero warnings
+            the_mean = the_sum / count
         ct_mask = count == 0
         if ct_mask.any():
             the_mean[ct_mask] = np.nan
diff --git a/pandas/tests/series/test_analytics.py b/pandas/tests/series/test_analytics.py
index 81d60aba44b0f..a3c61e330c37c 100644
--- a/pandas/tests/series/test_analytics.py
+++ b/pandas/tests/series/test_analytics.py
@@ -517,9 +517,10 @@ def _check_stat_op(self, name, alternate, string_series_,
             string_series_[5:15] = np.NaN
 
             # idxmax, idxmin, min, and max are valid for dates
-            if name not in ['max', 'min']:
+            if name not in ['max', 'min', 'mean']:
                 ds = Series(date_range('1/1/2001', periods=10))
-                pytest.raises(TypeError, f, ds)
+                with pytest.raises(TypeError):
+                    f(ds)
 
             # skipna or no
             assert notna(f(string_series_))
diff --git a/pandas/tests/test_nanops.py b/pandas/tests/test_nanops.py
index e214d4c1985a9..1e08914811402 100644
--- a/pandas/tests/test_nanops.py
+++ b/pandas/tests/test_nanops.py
@@ -14,6 +14,7 @@
 from pandas import Series, isna
 from pandas.compat.numpy import _np_version_under1p13
 from pandas.core.dtypes.common import is_integer_dtype
+from pandas.core.arrays import DatetimeArrayMixin as DatetimeArray
 
 use_bn = nanops._USE_BOTTLENECK
 
@@ -998,6 +999,23 @@ def prng(self):
         return np.random.RandomState(1234)
 
 
+class TestDatetime64NaNOps(object):
+    @pytest.mark.parametrize('tz', [None, 'UTC'])
+    def test_nanmean(self, tz):
+        dti = pd.date_range('2016-01-01', periods=3, tz=tz)
+        expected = dti[1]
+
+        for obj in [dti, DatetimeArray(dti), Series(dti)]:
+            result = nanops.nanmean(obj)
+            assert result == expected
+
+        dti2 = dti.insert(1, pd.NaT)
+
+        for obj in [dti2, DatetimeArray(dti2), Series(dti2)]:
+            result = nanops.nanmean(obj)
+            assert result == expected
+
+
 def test_use_bottleneck():
 
     if nanops._BOTTLENECK_INSTALLED:


Code review comments:
diff_hunk: @@ -483,6 +483,26 @@ def _maybe_mask_results(self, result, fill_value=iNaT, convert=None):
             result[self._isnan] = fill_value
         return result
 
+    # ------------------------------------------------------------------
+    # Additional array methods
+    # These are not part of the EA API, but we implement them because
+    # pandas currently assumes they're there.
+
+    def view(self, dtype=None):
path: pandas/core/arrays/datetimelike.py
text: @user2:
are we adding to EA interface? or just here (ok either way). @user1 ?

@user1:
I don't think ExtensionArray.view makes sense. The result of that would (I think) depend on the memory layout of the concrete array backing the ExtesnionArray, and I think that ExtensionArray should remain agnostic to that.

diff_hunk: @@ -951,22 +957,36 @@ def max(self):
         >>> idx.max()
         ('b', 2)
         """
+        nv.validate_minmax_axis(axis)
         return nanops.nanmax(self.values)
 
-    def argmax(self, axis=None):
+    def argmax(self, axis=None, skipna=True):
         """
         Return a ndarray of the maximum argument indexer.
 
+        Parameters
+        ----------
+        axis : {None}
+            Dummy argument for consistency with Series
+        skipna : bool, default True
+
         See Also
         --------
         numpy.ndarray.argmax
         """
+        nv.validate_minmax_axis(axis)
         return nanops.nanargmax(self.values)
path: pandas/core/base.py
text: @user1:
Doesn't this hit https://github.com/pandas-dev/pandas/issues/24265? 

@author:
Not this line, but the relevant line in indexes.datetimelike does.  With the commit just pushed, this fixed 24265

diff_hunk: @@ -951,22 +957,36 @@ def max(self):
         >>> idx.max()
         ('b', 2)
         """
+        nv.validate_minmax_axis(axis)
         return nanops.nanmax(self.values)
path: pandas/core/base.py
text: @user1:
Why isn't skipna passed? Is that a followup item?

@author:
I had considered this a follow-up item, but am now leaning towards fixing these now.  Good catch.  

@author:
This turns out to be more involved than expected, requires some changes in Series._reduce and IndexOpsMixin._reduce.  Neither of which is all that invasive, but changing them entails changing/fixing/testing behavior for other reductions I hadn't planned on doing in this PR.

Currently going through to get that all done here.  I'm also OK with doing this in two phases so as to take things out of #24024 sooner rather than later.