From dcce77c74545b2adc9f22a036f321ae6e39bb57b Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Mon, 22 Oct 2018 20:34:24 -0700 Subject: [PATCH 1/9] BUG-23282 calling min on series of NaT returns NaT --- pandas/core/nanops.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index 2884bc1a19491..f07f054c2216b 100644 --- a/pandas/core/nanops.py +++ b/pandas/core/nanops.py @@ -718,6 +718,8 @@ def reduction(values, axis=None, skipna=True, mask=None): result = np.nan else: result = getattr(values, meth)(axis) + if is_integer(result) and result == _int64_max: + result = tslibs.iNaT result = _wrap_results(result, dtype) return _maybe_null_out(result, axis, mask) From ef91c24b93952646a4986ad5f3bdfb48f6e3026f Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Mon, 22 Oct 2018 20:50:44 -0700 Subject: [PATCH 2/9] BUG-23282 Add tests --- pandas/tests/series/test_datetime_values.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pandas/tests/series/test_datetime_values.py b/pandas/tests/series/test_datetime_values.py index e06d3a67db662..cd6a81230492c 100644 --- a/pandas/tests/series/test_datetime_values.py +++ b/pandas/tests/series/test_datetime_values.py @@ -509,3 +509,8 @@ def test_dt_timetz_accessor(self, tz_naive_fixture): time(22, 14, tzinfo=tz)]) result = s.dt.timetz tm.assert_series_equal(result, expected) + + def test_minmax_nat(self): + series = pd.Series([pd.NaT, pd.NaT]) + assert series.min() is pd.NaT + assert series.max() is pd.NaT From 8caeaedfe01a1f726c5899c3b85da552e301ec8a Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Mon, 22 Oct 2018 23:02:48 -0700 Subject: [PATCH 3/9] BUG-23282 Add additional tests --- pandas/core/nanops.py | 3 ++- pandas/tests/series/test_datetime_values.py | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index f07f054c2216b..83067ffd64c79 100644 --- a/pandas/core/nanops.py +++ b/pandas/core/nanops.py @@ -718,7 +718,8 @@ def reduction(values, axis=None, skipna=True, mask=None): result = np.nan else: result = getattr(values, meth)(axis) - if is_integer(result) and result == _int64_max: + if (is_integer(result) and is_datetime_or_timedelta_dtype(dtype) + and result == _int64_max): result = tslibs.iNaT result = _wrap_results(result, dtype) diff --git a/pandas/tests/series/test_datetime_values.py b/pandas/tests/series/test_datetime_values.py index cd6a81230492c..1fb7de3730b55 100644 --- a/pandas/tests/series/test_datetime_values.py +++ b/pandas/tests/series/test_datetime_values.py @@ -510,7 +510,18 @@ def test_dt_timetz_accessor(self, tz_naive_fixture): result = s.dt.timetz tm.assert_series_equal(result, expected) - def test_minmax_nat(self): - series = pd.Series([pd.NaT, pd.NaT]) - assert series.min() is pd.NaT - assert series.max() is pd.NaT + @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): + assert nat.min() is pd.NaT + assert nat.max() is pd.NaT + + @pytest.mark.parametrize('nat', [ + 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 From ce0a7ee2a2ec7fdd1383c7d3f160b880e46062eb Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Tue, 23 Oct 2018 11:30:17 -0700 Subject: [PATCH 4/9] BUG-23282 Move check to _wrap_result and add bug number to tests --- pandas/core/nanops.py | 6 +++--- pandas/tests/series/test_datetime_values.py | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index 83067ffd64c79..36881fb6c6232 100644 --- a/pandas/core/nanops.py +++ b/pandas/core/nanops.py @@ -269,6 +269,9 @@ def _view_if_needed(values): def _wrap_results(result, dtype): """ wrap our results if needed """ + if (is_integer(result) and is_datetime_or_timedelta_dtype(dtype) + and result == _int64_max): + result = tslibs.iNaT if is_datetime64_dtype(dtype): if not isinstance(result, np.ndarray): result = tslibs.Timestamp(result) @@ -718,9 +721,6 @@ def reduction(values, axis=None, skipna=True, mask=None): result = np.nan else: result = getattr(values, meth)(axis) - if (is_integer(result) and is_datetime_or_timedelta_dtype(dtype) - and result == _int64_max): - result = tslibs.iNaT result = _wrap_results(result, dtype) return _maybe_null_out(result, axis, mask) diff --git a/pandas/tests/series/test_datetime_values.py b/pandas/tests/series/test_datetime_values.py index 1fb7de3730b55..7c74da8e2d684 100644 --- a/pandas/tests/series/test_datetime_values.py +++ b/pandas/tests/series/test_datetime_values.py @@ -515,10 +515,12 @@ def test_dt_timetz_accessor(self, tz_naive_fixture): 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')])]) From a1bde9c0720146dd5f3ccdddac77c289eaa18fd2 Mon Sep 17 00:00:00 2001 From: Jeff Reback Date: Wed, 24 Oct 2018 08:04:27 -0400 Subject: [PATCH 5/9] use fill_value directly --- pandas/core/nanops.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index 36881fb6c6232..7d25a7cba0596 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,19 +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_integer(result) and is_datetime_or_timedelta_dtype(dtype) - and result == _int64_max): - result = tslibs.iNaT 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: @@ -451,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): @@ -504,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 @@ -708,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 @@ -722,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 @@ -756,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 @@ -786,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 From 9d6dca3dacc533da7e45c0dbfa597d6a6504b57c Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Thu, 25 Oct 2018 13:20:20 -0700 Subject: [PATCH 6/9] BUG-23282 add more underscores --- pandas/core/nanops.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index 7d25a7cba0596..93b781199f22d 100644 --- a/pandas/core/nanops.py +++ b/pandas/core/nanops.py @@ -350,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) @@ -383,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) @@ -413,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 From 314b2bc1bcfa93b81635abc5a0ab041d7b9673be Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Thu, 25 Oct 2018 16:07:30 -0700 Subject: [PATCH 7/9] BUG-23282 Fix linting --- pandas/core/nanops.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index 93b781199f22d..38c83fd6e6771 100644 --- a/pandas/core/nanops.py +++ b/pandas/core/nanops.py @@ -351,7 +351,7 @@ def nanany(values, axis=None, skipna=True, mask=None): False """ values, mask, dtype, _, _ = _get_values(values, skipna, False, copy=skipna, - mask=mask) + mask=mask) return values.any(axis) @@ -384,7 +384,7 @@ def nanall(values, axis=None, skipna=True, mask=None): False """ values, mask, dtype, _, _ = _get_values(values, skipna, True, copy=skipna, - mask=mask) + mask=mask) return values.all(axis) @@ -413,7 +413,8 @@ 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 From 15e5e8798ba283fd9e39adf7548f4d809ea25f22 Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Thu, 25 Oct 2018 20:43:41 -0700 Subject: [PATCH 8/9] BUG-23282 Add whatsnew and asserts --- doc/source/whatsnew/v0.24.0.txt | 1 + pandas/core/nanops.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index a41b0c9521f99..cfd9032caa71a 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -999,6 +999,7 @@ Datetimelike - Bug in :func:`to_datetime` with an :class:`Index` argument that would drop the ``name`` from the result (:issue:`21697`) - Bug in :class:`PeriodIndex` where adding or subtracting a :class:`timedelta` or :class:`Tick` object produced incorrect results (:issue:`22988`) - Bug in :func:`date_range` when decrementing a start date to a past end date by a negative frequency (:issue:`23270`) +- Bug in :func:`min` which would return ``NaN`` instead of ``NaT`` when called on a series of ``NaT`` (:issue:`23282`) Timedelta ^^^^^^^^^ diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index 38c83fd6e6771..d7314355476f6 100644 --- a/pandas/core/nanops.py +++ b/pandas/core/nanops.py @@ -271,6 +271,7 @@ def _wrap_results(result, dtype, fill_value=None): if is_datetime64_dtype(dtype): if not isinstance(result, np.ndarray): + assert not isna(fill_value), "Expected non-null fill_value" if result == fill_value: result = np.nan result = tslibs.Timestamp(result) @@ -278,6 +279,7 @@ def _wrap_results(result, dtype, fill_value=None): result = result.view(dtype) elif is_timedelta64_dtype(dtype): if not isinstance(result, np.ndarray): + assert not isna(fill_value), "Expected non-null fill_value" if result == fill_value: result = np.nan From 815da44bce3b64611d8abba9603204d0719496be Mon Sep 17 00:00:00 2001 From: JustinZhengBC Date: Fri, 26 Oct 2018 14:58:02 -0700 Subject: [PATCH 9/9] BUG-23282 Fix whatsnew and assert --- doc/source/whatsnew/v0.24.0.txt | 2 +- pandas/core/nanops.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index cfd9032caa71a..ffb81c79f3434 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -999,7 +999,7 @@ Datetimelike - Bug in :func:`to_datetime` with an :class:`Index` argument that would drop the ``name`` from the result (:issue:`21697`) - Bug in :class:`PeriodIndex` where adding or subtracting a :class:`timedelta` or :class:`Tick` object produced incorrect results (:issue:`22988`) - Bug in :func:`date_range` when decrementing a start date to a past end date by a negative frequency (:issue:`23270`) -- Bug in :func:`min` which would return ``NaN`` instead of ``NaT`` when called on a series of ``NaT`` (:issue:`23282`) +- Bug in :meth:`Series.min` which would return ``NaN`` instead of ``NaT`` when called on a series of ``NaT`` (:issue:`23282`) Timedelta ^^^^^^^^^ diff --git a/pandas/core/nanops.py b/pandas/core/nanops.py index d7314355476f6..afba433f0e391 100644 --- a/pandas/core/nanops.py +++ b/pandas/core/nanops.py @@ -279,7 +279,6 @@ def _wrap_results(result, dtype, fill_value=None): result = result.view(dtype) elif is_timedelta64_dtype(dtype): if not isinstance(result, np.ndarray): - assert not isna(fill_value), "Expected non-null fill_value" if result == fill_value: result = np.nan