-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG GH23282 calling min on series of NaT returns NaT #23289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
dcce77c
ef91c24
8caeaed
ce0a7ee
123ff25
a1bde9c
9d6dca3
314b2bc
15e5e87
e45d908
815da44
a70b903
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,22 @@ 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): | ||
assert not isna(fill_value), "Expected non-null fill_value" | ||
if result == fill_value: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it assumed that Should we assert that it's not NA? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so this can't be for an i8 type by definition. but yes we can assert it. |
||
result = np.nan | ||
result = tslibs.Timestamp(result) | ||
else: | ||
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 | ||
|
||
# raise if we have a timedelta64[ns] which is too large | ||
if np.fabs(result) > _int64_max: | ||
|
@@ -346,8 +352,8 @@ def nanany(values, axis=None, skipna=True, mask=None): | |
>>> nanops.nanany(s) | ||
False | ||
""" | ||
values, mask, dtype, _ = _get_values(values, skipna, False, copy=skipna, | ||
mask=mask) | ||
values, mask, dtype, _, _ = _get_values(values, skipna, False, copy=skipna, | ||
mask=mask) | ||
return values.any(axis) | ||
|
||
|
||
|
@@ -379,8 +385,8 @@ def nanall(values, axis=None, skipna=True, mask=None): | |
>>> nanops.nanall(s) | ||
False | ||
""" | ||
values, mask, dtype, _ = _get_values(values, skipna, True, copy=skipna, | ||
mask=mask) | ||
values, mask, dtype, _, _ = _get_values(values, skipna, True, copy=skipna, | ||
mask=mask) | ||
return values.all(axis) | ||
|
||
|
||
|
@@ -409,7 +415,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 | ||
|
@@ -448,7 +455,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 +509,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 +713,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 +728,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 +762,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 +792,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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the bug in the builtin
min
from the standard library, orSeries.min
? Right now, you're linking to the builtin.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was to
Series.min
. I think it's fixed now