BUG: Min/max does not work for dates with timezones if there are missing values in the data frame
State: closed

- [x] closes #44196 and #27794
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit) for how to run them
- [x] whatsnew entry

This is an attempt at fixing the bug of wrong `min`/`max` aggregation with timezone aware data and at least one `pd.NaT` in the data to aggregate. While I still see quite a lot of weaknesses in my approach I want to elaborate why I did what I did, what the weaknesses of this approach are, how this might be handled better and why I didn't follow that path.

#### Problem description
To fully understand the problem at hand, let's look at three different cases:

```python
import pandas as pd
# CASE 1
# No Problem without NaT
rng_with_tz = pd.date_range(start='2021-10-01T12:00:00+02:00', end='2021-10-02T12:00:00+02:00', freq='4H')
df_with_tz = pd.DataFrame(data={'A': rng_with_tz, 'B': rng_with_tz + pd.Timedelta(minutes=20)})
df_with_tz.max(axis=1)

# No problem with timezone naive dataframe
# CASE 2
rng_tz_naive = pd.date_range(start='2021-10-01T12:00:00', end='2021-10-02T12:00:00', freq='4H')
df_tz_naive = pd.DataFrame(data={'A': rng_tz_naive, 'B': rng_tz_naive + pd.Timedelta(minutes=20)})
df_tz_naive.iloc[2, 1] = pd.NaT
df_tz_naive.max(axis=1)

# Incorrect result and warning with timezone aware df
# CASE 1
rng_with_tz = pd.date_range(start='2021-10-01T12:00:00+02:00', end='2021-10-02T12:00:00+02:00', freq='4H')
df_with_tz = pd.DataFrame(data={'A': rng_with_tz, 'B': rng_with_tz + pd.Timedelta(minutes=20)})
df_with_tz.iloc[2, 1] = pd.NaT
df_with_tz.max(axis=1)
```

When one drills down into the first case no problems arise. The `max` call is handled down to [`pandas/core/nanops.py:reduction`](https://github.com/pandas-dev/pandas/blob/master/pandas/core/nanops.py#L1023) and is running through fine. In the second case one finds that in [`pandas/core/nanops.py:reduction`](https://github.com/pandas-dev/pandas/blob/master/pandas/core/nanops.py#L1023) right after the call to [`pandas/core/nanops.py:_get_values`](https://github.com/pandas-dev/pandas/blob/66ce5de55b4aa6d4ca8bef65318cc9e2381689f6/pandas/core/nanops.py#L257) one realizes that all values are cast to integers, even the `pd.NaT`. 
In the third case one can see that the values aren't casted and `pd.NaT` is replaced by `-np.inf` which comes from [this line](https://github.com/pandas-dev/pandas/blob/66ce5de55b4aa6d4ca8bef65318cc9e2381689f6/pandas/core/nanops.py#L1051). This replacement is the reason for our error:
```python
TypeError: '>=' not supported between instances of 'Timestamp' and 'float'
```
when we try to [apply the max from numpy](https://github.com/pandas-dev/pandas/blob/66ce5de55b4aa6d4ca8bef65318cc9e2381689f6/pandas/core/nanops.py#L1042)
which results from comparing `pd.Timestamp`s with `float`s (`np.inf`). This error is caught and all elements are converted into `np.nan`.
The reason behind the error is, that
```python
>>> import pandas as pd

>>> rng_with_tz = pd.date_range(start='2021-10-01T12:00:00+02:00', end='2021-10-02T12:00:00+02:00', freq='4H')
>>> rng_with_tz
DatetimeIndex(['2021-10-01 12:00:00+02:00', '2021-10-01 16:00:00+02:00',
               '2021-10-01 20:00:00+02:00', '2021-10-02 00:00:00+02:00',
               '2021-10-02 04:00:00+02:00', '2021-10-02 08:00:00+02:00',
               '2021-10-02 12:00:00+02:00'],
              dtype='datetime64[ns, pytz.FixedOffset(120)]', freq='4H')
>>> rng_with_tz.values.dtype
dtype('<M8[ns]')
 
>>> rng_with_tz = pd.date_range(start='2021-10-01T12:00:00+02:00', end='2021-10-02T12:00:00+02:00', freq='4H')
>>> df_with_tz = pd.DataFrame(data={'A': rng_with_tz, 'B': rng_with_tz + pd.Timedelta(minutes=20)})
>>> df_with_tz.values.dtype
dtype('O')
```
the `dtype` changes when using timezone aware data, though the `dtype`s of the single series is still `dtype('<M8[ns]')`. This has serious implications: In [`pandas/core/nanops.py:_get_values`](https://github.com/pandas-dev/pandas/blob/66ce5de55b4aa6d4ca8bef65318cc9e2381689f6/pandas/core/nanops.py#L257) the `if` condition in this line (https://github.com/pandas-dev/pandas/blob/66ce5de55b4aa6d4ca8bef65318cc9e2381689f6/pandas/core/nanops.py#L313) is `False` and no cast to happens but still the `pd.NaT` is replaced by `np.inf` which is the reason for the error stated above.

#### Possible improvements on this PR
If we could change the `dtype` of the timezone aware DataFrame with missing values to the `numpy` dtype: `dtype('<M8[ns]')` we should be fine. But I have no idea how we can achieve that, since these are not `pandas` but `numpy` types we are talking about and I didn't really understand how `numpy` determines it`s types.

#### Weaknesses of this PR
The assumption made while catching the exceptions aren't well tested (though all tests are passed) and there might be a better and more generic solution.

Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff
name: Stale

Comments:
@jbrockmendel one more thing: I don't quite understand how working blockwise could have solved this problem as [you hinted](https://github.com/pandas-dev/pandas/issues/27794#issuecomment-698084941). I checked the blocks and one of the blocks also contains the `pd.NaT` value with `dtype('O')` and the same problem will arise here. Could you give me an idea how this might work?
> Could you give me an idea how this might work?

bc we don't fully support 2D EAs (@jreback: drink!), calling .values on a DataFrame with dt64tz dtypes upcasts to object, at which point things go off the rails.  If we were to operate directly on the blocks (as happens in the axis=0 case when we call `df._mgr.reduce`) this would work correctly.

Three options come to mind here:

1) recognize that the numeric_only=None case is deprecated so this use case will go away completely in 2.0.  Live with this bug in the interim.

2) Patch the affected nanops function to handle NaT correctly when it sees object-dtype (best guess is this will involve passing `initial` to np.minimum.reduce` circa nanops L1042

3) Re-write DataFrame._reduce axis=1 case to first iterate over `self._mgr.arrays` calling blk_func, then do the reduction on the resulting column arrays.
Hello @CloseChoice! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:







There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2021-11-07 16:08:07 UTC
@github-actions pre-commit
@jbrockmendel: I rewrote the reduce function for the `min` and `max` cases but had to adjust some tests, since we weren't consistent before, e.g.

```python
import pandas as pd
from pandas import Timedelta, Timestamp

# test_oerators_timedelta64
df = pd.DataFrame({'A': {0: Timedelta('0 days 00:05:05'), 1: Timedelta('1 days 00:05:05'), 2: Timedelta('2 days 00:05:05')},
                   'B': {0: Timedelta('-1 days +00:00:00'), 1: Timedelta('-1 days +00:00:00'), 2: Timedelta('-1 days +00:00:00')},
                   'C': {0: 'foo', 1: 'foo', 2: 'foo'},
                   'D': {0: 1, 1: 1, 2: 1},
                   'E': {0: 1.0, 1: 1.0, 2: 1.0},
                   'F': {0: Timestamp('2013-01-01 00:00:00'), 1: Timestamp('2013-01-01 00:00:00'), 2: Timestamp('2013-01-01 00:00:00')}})
# the following two lines should result in the same output but this isn't the case on master
df.max(axis=1)
df.T.max(axis=0)
```

This would also be fixed by the current state of the PR. Not sure it would be possible to rewrite all reduction operations like this but I could give it a try. Currently one test will fail which is connected issue xref #44336
@github-actions pre-commit
have you tried looking at core.nanops._get_values?  If I disable the masking L328-334 for object dtype, the motivating case works.  (15 other tests fail, so there's some more kinks to work out)
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
> have you tried looking at core.nanops._get_values? If I disable the masking L328-334 for object dtype, the motivating case works. (15 other tests fail, so there's some more kinks to work out)

can you try this
iirc this was close if you can merge main and move the note to 1.5
Thanks for the PR, but it appears to have gone stale. If interested in continuing please merge the main branch, address the review, and we can reopen.