Skip to content

BUG: DataFrame.min/max with axis=1 and uniform datetime64[ns, tz] types does not return NaNs #24759

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

Merged
merged 13 commits into from
Jan 16, 2019
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.24.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1647,6 +1647,7 @@ Timezones
- Bug in :meth:`DataFrame.any` returns wrong value when ``axis=1`` and the data is of datetimelike type (:issue:`23070`)
- Bug in :meth:`DatetimeIndex.to_period` where a timezone aware index was converted to UTC first before creating :class:`PeriodIndex` (:issue:`22905`)
- Bug in :meth:`DataFrame.tz_localize`, :meth:`DataFrame.tz_convert`, :meth:`Series.tz_localize`, and :meth:`Series.tz_convert` where ``copy=False`` would mutate the original argument inplace (:issue:`6326`)
- Bug in :meth:`DataFrame.max` and :meth:`DataFrame.min` with ``axis=1`` where a :class:`Series` with ``NaN`` would be returned when all columns contained the same timezone (:issue:`10390`)

Offsets
^^^^^^^
Expand Down
4 changes: 3 additions & 1 deletion pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -7390,7 +7390,9 @@ def f(x):
return op(x, axis=axis, skipna=skipna, **kwds)

# exclude timedelta/datetime unless we are uniform types
if axis == 1 and self._is_mixed_type and self._is_datelike_mixed_type:
if (axis == 1 and self._is_datelike_mixed_type
and (self._is_mixed_type
and not self._is_all_same_datetimetz_types)):
numeric_only = True

if numeric_only is None:
Expand Down
5 changes: 5 additions & 0 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -5164,6 +5164,11 @@ def _is_datelike_mixed_type(self):
f = lambda: self._data.is_datelike_mixed_type
return self._protect_consolidate(f)

@property
Copy link
Contributor

Choose a reason for hiding this comment

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

you shouldn't need this as _is_datelike_mixed_type should cover this case, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

_is_datelike_mixed_type is not specific to tz-aware blocks nor checks the actual timezone which we need here

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you instead use self._is_homogenous_type and a check that any (or the first) dtype is datetimetz?

def _is_all_same_datetimetz_types(self):
f = lambda: self._data.all_same_datetimetz_types
return self._protect_consolidate(f)

def _check_inplace_setting(self, value):
""" check whether we allow in-place setting with this type of value """

Expand Down
10 changes: 10 additions & 0 deletions pandas/core/internals/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,16 @@ def is_view(self):

return False

@property
def all_same_datetimetz_types(self):
"""Whether all blocks are datetimetz and the same timezone"""
dtypes = set()
for block in self.blocks:
if not block.is_datetimetz or len(dtypes) > 1:
return False
dtypes.add(block.dtype)
return len(dtypes) == 1

def get_bool_data(self, copy=False):
"""
Parameters
Expand Down
12 changes: 12 additions & 0 deletions pandas/tests/frame/test_timezones.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,15 @@ def test_tz_localize_convert_copy_inplace_mutate(self, copy, method, tz):
index=date_range('20131027', periods=5,
freq='1H', tz=tz))
tm.assert_frame_equal(result, expected)

Copy link
Contributor

Choose a reason for hiding this comment

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

these should go with the appropriate tests, I think in test_nanops or maybe tests/reductions)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Moved to tests/reductions

@pytest.mark.parametrize('op, expected_col', [
['max', 'a'], ['min', 'b']
])
def test_same_tz_min_max_axis_1(self, op, expected_col):
# GH 10390
df = DataFrame(date_range('2016-01-01 00:00:00', periods=3, tz='UTC'),
columns=['a'])
df['b'] = df.a.subtract(pd.Timedelta(seconds=3600))
result = getattr(df, op)(axis=1)
expected = df[expected_col]
tm.assert_series_equal(result, expected)