Repository: pandas-dev/pandas
Pull request: BUG GH23282 calling min on series of NaT returns NaT
- [X] closes #23282 
- [X] tests passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

For `max`, `NaT` values are filled with the lowest possible value. For `min`, they are filled with the highest possible value. The problem is that only the lowest possible value is recognized as `NaT`. Since `nanops.py` is responsible for assigning the highest value to `NaT` when min is called, it should also be responsible for translating it to `NaT` when appropriate.

Problem statement:
.min() on a series of NaTs returns nan, while .max() returns NaT
#### Code Sample

```python
s = pd.Series([pd.NaT, pd.NaT])

# Returns nan
s.min()

# Returns NaT
s.max()

```
#### Problem description

Calling `.min()` on an series of `NaT` returns `nan`. I would expect this to return `NaT` to as to keep the type the same. Also, whether it returns `nan` or `NaT`, I would expect it to be consistent with `.max()`. I'm on version `0.22`, not `0.23`, so maybe it was fixed in the latest version, but I didn't see an issue addressing it.

#### Expected Output

I would expect that the `s.min()` call would also return `NaT`.

#### Output of ``pd.show_versions()``

<details>

INSTALLED VERSIONS
------------------
commit: None
python: 3.5.4.final.0
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 45 Stepping 7, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: None.None

pandas: 0.22.0
pytest: None
pip: 10.0.1
setuptools: 40.2.0
Cython: None
numpy: 1.15.1
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: 6.5.0
sphinx: None
patsy: None
dateutil: 2.7.3
pytz: 2018.5
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: None
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: 1.0.1
sqlalchemy: 1.2.11
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>

Hints from issue discussion:
Thanks for the report. I am seeing this behavior on master too. Also looks similar to https://github.com/pandas-dev/pandas/issues/10390.

Investigations and PRs welcome!

Commit message: BUG-23282 add more underscores

Patch to review:
diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py
index 2884bc1a19491..93b781199f22d 100644
--- a/pandas/core/nanops.py
+++ b/pandas/core/nanops.py
@@ -244,7 +244,7 @@ def _get_values(values, skipna, fill_value=None, fill_value_typ=None,
     elif is_float_dtype(dtype):
         dtype_max = np.float64
 
-    return values, mask, dtype, dtype_max
+    return values, mask, dtype, dtype_max, fill_value
 
 
 def _isfinite(values):
@@ -266,16 +266,20 @@ def _view_if_needed(values):
     return values
 
 
-def _wrap_results(result, dtype):
+def _wrap_results(result, dtype, fill_value=None):
     """ wrap our results if needed """
 
     if is_datetime64_dtype(dtype):
         if not isinstance(result, np.ndarray):
+            if result == fill_value:
+                result = np.nan
             result = tslibs.Timestamp(result)
         else:
             result = result.view(dtype)
     elif is_timedelta64_dtype(dtype):
         if not isinstance(result, np.ndarray):
+            if result == fill_value:
+                result = np.nan
 
             # raise if we have a timedelta64[ns] which is too large
             if np.fabs(result) > _int64_max:
@@ -346,7 +350,7 @@ def nanany(values, axis=None, skipna=True, mask=None):
     >>> nanops.nanany(s)
     False
     """
-    values, mask, dtype, _ = _get_values(values, skipna, False, copy=skipna,
+    values, mask, dtype, _, _ = _get_values(values, skipna, False, copy=skipna,
                                          mask=mask)
     return values.any(axis)
 
@@ -379,7 +383,7 @@ def nanall(values, axis=None, skipna=True, mask=None):
     >>> nanops.nanall(s)
     False
     """
-    values, mask, dtype, _ = _get_values(values, skipna, True, copy=skipna,
+    values, mask, dtype, _, _ = _get_values(values, skipna, True, copy=skipna,
                                          mask=mask)
     return values.all(axis)
 
@@ -409,7 +413,7 @@ def nansum(values, axis=None, skipna=True, min_count=0, mask=None):
     >>> nanops.nansum(s)
     3.0
     """
-    values, mask, dtype, dtype_max = _get_values(values, skipna, 0, mask=mask)
+    values, mask, dtype, dtype_max, _ = _get_values(values, skipna, 0, mask=mask)
     dtype_sum = dtype_max
     if is_float_dtype(dtype):
         dtype_sum = dtype
@@ -448,7 +452,8 @@ def nanmean(values, axis=None, skipna=True, mask=None):
     >>> nanops.nanmean(s)
     1.5
     """
-    values, mask, dtype, dtype_max = _get_values(values, skipna, 0, mask=mask)
+    values, mask, dtype, dtype_max, _ = _get_values(
+        values, skipna, 0, mask=mask)
     dtype_sum = dtype_max
     dtype_count = np.float64
     if is_integer_dtype(dtype) or is_timedelta64_dtype(dtype):
@@ -501,7 +506,7 @@ def get_median(x):
             return np.nan
         return np.nanmedian(x[mask])
 
-    values, mask, dtype, dtype_max = _get_values(values, skipna, mask=mask)
+    values, mask, dtype, dtype_max, _ = _get_values(values, skipna, mask=mask)
     if not is_float_dtype(values):
         values = values.astype('f8')
         values[mask] = np.nan
@@ -705,7 +710,8 @@ def nansem(values, axis=None, skipna=True, ddof=1, mask=None):
 def _nanminmax(meth, fill_value_typ):
     @bottleneck_switch()
     def reduction(values, axis=None, skipna=True, mask=None):
-        values, mask, dtype, dtype_max = _get_values(
+
+        values, mask, dtype, dtype_max, fill_value = _get_values(
             values, skipna, fill_value_typ=fill_value_typ, mask=mask)
 
         if ((axis is not None and values.shape[axis] == 0) or
@@ -719,7 +725,7 @@ def reduction(values, axis=None, skipna=True, mask=None):
         else:
             result = getattr(values, meth)(axis)
 
-        result = _wrap_results(result, dtype)
+        result = _wrap_results(result, dtype, fill_value)
         return _maybe_null_out(result, axis, mask)
 
     reduction.__name__ = 'nan' + meth
@@ -753,8 +759,8 @@ def nanargmax(values, axis=None, skipna=True, mask=None):
     >>> nanops.nanargmax(s)
     4
     """
-    values, mask, dtype, _ = _get_values(values, skipna, fill_value_typ='-inf',
-                                         mask=mask)
+    values, mask, dtype, _, _ = _get_values(
+        values, skipna, fill_value_typ='-inf', mask=mask)
     result = values.argmax(axis)
     result = _maybe_arg_null_out(result, axis, mask, skipna)
     return result
@@ -783,8 +789,8 @@ def nanargmin(values, axis=None, skipna=True, mask=None):
     >>> nanops.nanargmin(s)
     0
     """
-    values, mask, dtype, _ = _get_values(values, skipna, fill_value_typ='+inf',
-                                         mask=mask)
+    values, mask, dtype, _, _ = _get_values(
+        values, skipna, fill_value_typ='+inf', mask=mask)
     result = values.argmin(axis)
     result = _maybe_arg_null_out(result, axis, mask, skipna)
     return result
diff --git a/pandas/tests/series/test_datetime_values.py b/pandas/tests/series/test_datetime_values.py
index e06d3a67db662..7c74da8e2d684 100644
--- a/pandas/tests/series/test_datetime_values.py
+++ b/pandas/tests/series/test_datetime_values.py
@@ -509,3 +509,21 @@ def test_dt_timetz_accessor(self, tz_naive_fixture):
                            time(22, 14, tzinfo=tz)])
         result = s.dt.timetz
         tm.assert_series_equal(result, expected)
+
+    @pytest.mark.parametrize('nat', [
+        pd.Series([pd.NaT, pd.NaT]),
+        pd.Series([pd.NaT, pd.Timedelta('nat')]),
+        pd.Series([pd.Timedelta('nat'), pd.Timedelta('nat')])])
+    def test_minmax_nat_series(self, nat):
+        # GH 23282
+        assert nat.min() is pd.NaT
+        assert nat.max() is pd.NaT
+
+    @pytest.mark.parametrize('nat', [
+        # GH 23282
+        pd.DataFrame([pd.NaT, pd.NaT]),
+        pd.DataFrame([pd.NaT, pd.Timedelta('nat')]),
+        pd.DataFrame([pd.Timedelta('nat'), pd.Timedelta('nat')])])
+    def test_minmax_nat_dataframe(self, nat):
+        assert nat.min()[0] is pd.NaT
+        assert nat.max()[0] is pd.NaT


Code review comments:
diff_hunk: @@ -346,7 +350,7 @@ def nanany(values, axis=None, skipna=True, mask=None):
     >>> nanops.nanany(s)
     False
     """
-    values, mask, dtype, _ = _get_values(values, skipna, False, copy=skipna,
+    values, mask, dtype, _, _ = _get_values(values, skipna, False, copy=skipna,
                                          mask=mask)
path: pandas/core/nanops.py
text: @user1:
listing error I would guess.

diff_hunk: @@ -266,16 +266,20 @@ def _view_if_needed(values):
     return values
 
 
-def _wrap_results(result, dtype):
+def _wrap_results(result, dtype, fill_value=None):
     """ wrap our results if needed """
 
     if is_datetime64_dtype(dtype):
         if not isinstance(result, np.ndarray):
+            if result == fill_value:
path: pandas/core/nanops.py
text: @user1:
Is it assumed that `fill_value` is not NA? If not, this will be wrong, since `fill_value` will never equal `fill_value`.

Should we assert that it's not NA?

@user2:
so this can't be for an i8 type by definition. but yes we can assert it.