Skip to content

API: make min/max on empty datetime df consistent with datetime serie… #33911

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
May 9, 2020
Merged

API: make min/max on empty datetime df consistent with datetime serie… #33911

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6c255d0
API: make min/max on empty datetime df consistent with datetime serie…
CloseChoice Apr 30, 2020
363898d
update nanops and tests as result of PR discussion
CloseChoice May 1, 2020
0d9a754
removed git tracking of test file
CloseChoice May 1, 2020
6e14ff8
removed git tracking of test file
CloseChoice May 1, 2020
60ea00f
added whatsnew entry
CloseChoice May 1, 2020
5e57597
move tests to test_analytics.py
CloseChoice May 1, 2020
c62921b
removed blank line at-end-of-file test_analytics.py
CloseChoice May 1, 2020
b958e29
Merge branch 'master' into API-consistent-empty-datetime-max-or-min
CloseChoice May 2, 2020
e309459
Merge branch 'master' of https://github.com/pandas-dev/pandas into AP…
CloseChoice May 3, 2020
77fd5c6
splitted tests, update comment in nanops and update whatsnew entry
CloseChoice May 7, 2020
9219ec3
update test_analytics for linting
CloseChoice May 7, 2020
6dfec8f
Merge branch 'master' of https://github.com/pandas-dev/pandas into AP…
CloseChoice May 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions pandas/core/nanops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,12 @@ def _na_for_min_count(
else:
assert axis is not None # assertion to make mypy happy
result_shape = values.shape[:axis] + values.shape[axis + 1 :]
result = np.empty(result_shape, dtype=values.dtype)
result.fill(fill_value)
# calling np.full with dtype parameter throws an ValueError when called
# with np.datetime64 and pd.NaT
try:
result = np.full(result_shape, fill_value, dtype=values.dtype)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what hits each of these branches?

Copy link
Member Author

@CloseChoice CloseChoice May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when np.full is called with the parameter dtype=np.datetime64 and fill_value=pd.NaT a ValueError ValueError: cannot convert float NaN to integer. In this case I don't call np.full without an explicit dtype. Giving the array result an explicit dtype does not have an effect on the DataFrame column but I thought it is cleaner if I at least try to give it the correct one. I hoped the comment explains this. But if not, I'm gonna make it gonna improve the comment.

except ValueError:
result = np.full(result_shape, fill_value)
return result


Expand Down
25 changes: 25 additions & 0 deletions pandas/tests/reductions/test_reductions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,31 @@ def test_minmax_nat_dataframe(self, nat_df):
assert nat_df.min(skipna=False)[0] is pd.NaT
assert nat_df.max(skipna=False)[0] is pd.NaT

def test_min_max_dt64_with_NaT(self):
# Calling the following sum functions returned an error for dataframes but
# returned NaT for series. These tests check that the API is consistent in
# min/max calls on empty Series/DataFrames. See GH:33704 for more
# information

df = pd.DataFrame(dict(x=pd.to_datetime([])))
expected_dt_series = pd.Series(pd.to_datetime([]))
# check axis 0
assert (df.min(axis=0).x is NaT) == (expected_dt_series.min() is NaT)
assert (df.max(axis=0).x is NaT) == (expected_dt_series.max() is NaT)

# check axis 1
tm.assert_series_equal(df.min(axis=1), expected_dt_series)
Copy link
Member Author

@CloseChoice CloseChoice May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to mention that calling min on axis=1 of an empty dataframe does not return the same as calling it on a Series, since this it currently not possible (and I don't think it is desired) with the implementation of reduce-operations. This test checks that when reducing over the index a Dataframe behaves like a series.

tm.assert_series_equal(df.max(axis=1), expected_dt_series)

df = pd.DataFrame(dict(x=[]))
expected_float_series = pd.Series([], dtype=float)
# check axis 0
assert np.isnan(df.min(axis=0).x) == np.isnan(expected_float_series.min())
assert np.isnan(df.max(axis=0).x) == np.isnan(expected_float_series.max())
# check axis 1
tm.assert_series_equal(df.min(axis=1), expected_float_series)
tm.assert_series_equal(df.min(axis=1), expected_float_series)

def test_min_max(self):
rng = pd.date_range("1/1/2000", "12/31/2000")
rng2 = rng.take(np.random.permutation(len(rng)))
Expand Down