Repository: pandas-dev/pandas
Pull request: BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395
- [x] closes #32395 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
assignment with datetime64[ns, UTC] raises TypeError
#### Code Sample

```python
# Your code here

import datetime, pandas as pd

data = [[datetime.datetime(2020, 2, 28, 13, 51, 27, tzinfo=datetime.timezone.utc)],
 [datetime.datetime(2020, 2, 28, 13, 51, 27, tzinfo=datetime.timezone.utc)],
 [datetime.datetime(2020, 2, 28, 13, 51, 27, tzinfo=datetime.timezone.utc)],
 [datetime.datetime(2020, 2, 28, 13, 51, 27, tzinfo=datetime.timezone.utc)]]
df = pd.DataFrame(data, columns=['cd'])     # dtype: datetime64[ns, UTC]
df2 = pd.DataFrame(index=df.index)

df2.loc[:,'cd2'] = df['cd']                  # no error
df2.loc[df.index,'cd3'] = df['cd']     # error
df2.loc[df.index,'cd2'] = df['cd']     # no error

```
####  Problem description 

If you try to assign datetime values (with zone and indexes) to a column, it will raise TypeError: data type not understood. 
No errors raise with index ':', or when the column already has the correct type. Note that this only happens if the datetime has zone information. With tzinfo=None, no errors occur.

#### Output of ``pd.show_versions()``

I noticed this bug in version 1.0.1
No errors occur in 0.25.3

INSTALLED VERSIONS
------------------
commit           : None
pandas           : 1.0.1
numpy            : 1.16.4
pytz             : 2019.1
dateutil         : 2.8.0
pip              : 20.0.1
setuptools       : 45.1.0
Cython           : None
pytest           : None
hypothesis       : None
sphinx           : 2.4.2
blosc            : None
feather          : None
xlsxwriter       : None
lxml.etree       : None
html5lib         : None
pymysql          : None
psycopg2         : 2.8.2 (dt dec pq3 ext lo64)
jinja2           : 2.10.1
IPython          : 7.8.0
pandas_datareader: None
bs4              : 4.8.2
bottleneck       : None
fastparquet      : None
gcsfs            : 0.6.0
lxml.etree       : None
matplotlib       : 3.1.0
numexpr          : None
odfpy            : None
openpyxl         : None
pandas_gbq       : None
pyarrow          : 0.16.0
pytables         : None
pytest           : None
pyxlsb           : None
s3fs             : None
scipy            : 1.2.0
sqlalchemy       : None
tables           : None
tabulate         : None
xarray           : None
xlrd             : None
xlwt             : None
xlsxwriter       : None
numba            : None

Hints from issue discussion:
Thanks for the report. Things look weird around https://github.com/pandas-dev/pandas/blob/f25ed6f2b1898627c4cf4cb68d8be96387142c4d/pandas/core/internals/blocks.py#L842-L857,

```
(Pdb) pp self
DatetimeBlock: 4 dtype: datetime64[ns]
(Pdb) pp value
<DatetimeArray>
['2020-02-28 13:51:27+00:00', '2020-02-28 13:51:27+00:00',
 '2020-02-28 13:51:27+00:00', '2020-02-28 13:51:27+00:00']
Length: 4, dtype: datetime64[ns, UTC]
(Pdb) pp dtype
<class 'object'>
(Pdb) pp self.values
array(['NaT', 'NaT', 'NaT', 'NaT'], dtype='datetime64[ns]')
```

I see a couple options, both of which are likely valuable. 

1. Ensure we create a `datetime64[ns, <tz>]` array of all-NA values, where the `tz` matches the target dtype
2. Ensure that Block.setitem correctly handles `datetime64[ns] -> datetime64[ns, tz]`.

Are you interested in working on this?
(tagged this as a regression)
In 0.25 the `DatetimeArray` value to be assigned was coerced to an array of `Timestamp` values when it was assigned to an `object` dtype, I think that that part of the code can be reintroduced.
```
    def _try_coerce_args(self, other):
        """ provide coercion to our input arguments """

        if isinstance(other, ABCDatetimeIndex):
            # May get a DatetimeIndex here. Unbox it.
            other = other.array

        if isinstance(other, DatetimeArray):
            # hit in pandas/tests/indexing/test_coercion.py
            # ::TestWhereCoercion::test_where_series_datetime64[datetime64tz]
            # when falling back to ObjectBlock.where
            other = other.astype(object)

        return other
```

I can work on this.

Commit message: Add dtype to series object in tests to suppress warning

Patch to review:
diff --git a/doc/source/whatsnew/v1.0.2.rst b/doc/source/whatsnew/v1.0.2.rst
index 808e6ae709ce9..44a6adb999c3d 100644
--- a/doc/source/whatsnew/v1.0.2.rst
+++ b/doc/source/whatsnew/v1.0.2.rst
@@ -23,6 +23,7 @@ Fixed regressions
 - Fixed regression where :func:`read_pickle` raised a ``UnicodeDecodeError`` when reading a py27 pickle with :class:`MultiIndex` column (:issue:`31988`).
 - Fixed regression in :class:`DataFrame` arithmetic operations with mis-matched columns (:issue:`31623`)
 - Fixed regression in :meth:`GroupBy.agg` calling a user-provided function an extra time on an empty input (:issue:`31760`)
+- Fixed regression in :meth:`loc.__setitem__` throwing an error when a datetime64[ns, tz] value is provided (:issue:`32395`)
 - Joining on :class:`DatetimeIndex` or :class:`TimedeltaIndex` will preserve ``freq`` in simple cases (:issue:`32166`)
 -
 
diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py
index 70fd3ecdc2098..738bae62dff99 100644
--- a/pandas/core/internals/blocks.py
+++ b/pandas/core/internals/blocks.py
@@ -839,6 +839,9 @@ def setitem(self, indexer, value):
 
         # coerce if block dtype can store value
         values = self.values
+        if is_object_dtype(values.dtype) and isinstance(value, DatetimeArray):
+            value = value.astype(object)
+
         if self._can_hold_element(value):
             # We only get here for non-Extension Blocks, so _try_coerce_args
             #  is only relevant for DatetimeBlock and TimedeltaBlock
diff --git a/pandas/tests/indexing/test_loc.py b/pandas/tests/indexing/test_loc.py
index 4d042af8d59b4..316c0df919b06 100644
--- a/pandas/tests/indexing/test_loc.py
+++ b/pandas/tests/indexing/test_loc.py
@@ -1,4 +1,5 @@
 """ test label based indexing with loc """
+from datetime import timezone
 from io import StringIO
 import re
 
@@ -975,3 +976,42 @@ def test_loc_mixed_int_float():
 
     result = ser.loc[1]
     assert result == 0
+
+
+def test_loc_setitem_df_datetime64tz_column_with_index():
+    df = pd.DataFrame(
+        pd.date_range("2020-01-01", "2020-01-06", 6, tz=timezone.utc), columns=["data"]
+    )
+    df2 = pd.DataFrame(index=df.index)
+    df2.loc[df.index, "data"] = df["data"]
+    tm.assert_numpy_array_equal(np.array(df.data), np.array(df2.data))
+    assert df2.data.dtype == np.object
+
+
+def test_loc_setitem_df_datetime64tz_column_without_index():
+    df = pd.DataFrame(
+        pd.date_range("2020-01-01", "2020-01-06", 6, tz=timezone.utc), columns=["data"]
+    )
+    df2 = pd.DataFrame(index=df.index)
+    df2.loc[:, "data"] = df["data"]
+    tm.assert_series_equal(df.data, df2.data)
+
+
+def test_loc_setitem_series_datetime64tz_with_index():
+    s1 = pd.Series(
+        pd.date_range("2020-01-01", "2020-01-06", 6, tz=timezone.utc), name="data"
+    )
+    s2 = pd.Series(index=s1.index, dtype=np.object, name="data")
+    s2.loc[s1.index] = s1
+    tm.assert_numpy_array_equal(np.array(s1), np.array(s2))
+    assert s2.dtype == np.object
+
+
+def test_loc_setitem_series_datetime64tz_without_index():
+    s1 = pd.Series(
+        pd.date_range("2020-01-01", "2020-01-06", 6, tz=timezone.utc), name="data"
+    )
+    s2 = pd.Series(index=s1.index, dtype=np.object, name="data")
+    s2.loc[:] = s1
+    tm.assert_numpy_array_equal(np.array(s1), np.array(s2))
+    assert s2.dtype == np.object


Code review comments:
diff_hunk: @@ -839,6 +839,9 @@ def setitem(self, indexer, value):
 
         # coerce if block dtype can store value
         values = self.values
+        if is_object_dtype(values.dtype) and isinstance(value, DatetimeArray):
path: pandas/core/internals/blocks.py
text: @user1:
Why is `values.dtype` object dtype here? That surprises me.

@author:
When a `datetime[ns, tz]` column is assigned to a new column using an index an empty column is created with fill values, for any `datetime` or `timedelta` type the fill value is `NaT` (`pandas.core.dtypes.missing._infer_fill_value`) with the corresponding dtype. For `datetime[ns, tz]` the fill value is still of dtype `datetime[ns]`. In `Block.setitem` the common dtype between `datetime[ns]` and `datetime[ns, tz]` is `object` and the block is converted to an `ObjectBlock` and its `setitem` method is called which leads to the `values.dtype` to be `object`.  You could change this behaviour for the specific case of `datetime[ns, tz]` by changing the fill value but what would the fill value be for creating an empty `datetime[ns, tz]` column?

@user1:
>  In Block.setitem the common dtype between datetime[ns] and datetime[ns, tz] is object and the block is converted to an ObjectBlock and its setitem method is called which leads to the values.dtype to be object

I think this is the issue. We should be able to go from `datetime[ns] -> datetime[ns, tz]` without going through object dtype. Fixing it earlier to fill NaT with datetime64[ns, tz] dtype would also be an option. I'm not sure which is preferable.

@author:
`NaT` itself has no timezone information so a `datetime[ns, tz]` `NaT` object cannot be created, the current fill code uses a numpy `datetime64` object which does not support timezone information. You could wrap the `NaT` in a `DatetimeIndex` to create a `datetime[ns, tz]` object but I do not think that is a good solution. I think that one solution going forward would be to support a  `datetime[ns, tz]` `NaT` object but that should be a separate discussion.

@user2:
> I think that one solution going forward would be to support a datetime[ns, tz] NaT object but that should be a separate discussion.

thats not unreasonable, but its also very unlikely, especially before 2.0

> You could wrap the NaT in a DatetimeIndex to create a datetime[ns, tz] object but I do not think that is a good solution

This sounds reasonable, can you expand on why you think it isnt a good solution?

@author:
> This sounds reasonable, can you expand on why you think it isnt a good solution?

The missing value is returned by `pandas.core.dtypes.missing._infer_fill_value`, the `missing` module is used in the definition of `DatetimeIndex` so you end up with a fix to the indexer `_setitem_with_indexer` looking something like this -
```
if hasattr(value, "dtype") and is_datetime64tz_dtype(value.dtype):
    self.obj[key] = DatetimeIndex(["NaT"] * len(value), tz=value.dtype.tz)
else:
    self.obj[key] = _infer_fill_value(value)
```
To me this does not seem to be a good way to fix the problem, it does however retain the dtype so if there is a consensus that this is the optimal solution for now I can add a commit with this modification.


@user2:
bit of a kludge, but what if we made _infer_fill_value return a length=1 DatetimeArray in this case?

@author:
`_infer_fill_value`  is in the `pandas.core.dtypes.missing` module, the `isna` function from that module is used in the definition of `DatetimeArray`. `_infer_fill_value` is only used in the `_setitem_with_indexer` function so this could be implemented by moving the `_infer_fill_value` function to the `pandas.core.indexing` module. The code would be quite similar - you would have to create a `DatetimeArray` the size of `value` for the assignment to work.

@user3:
why are you doing this in blocks generically?

this should only be in an Object block. 

@author:
@user3 yes, I can move this, I was waiting for an update on https://github.com/pandas-dev/pandas/pull/32479#discussion_r392056376 and https://github.com/pandas-dev/pandas/pull/32479#discussion_r395987552 alternate solutions that do not require this code. Also relevant for your observation https://github.com/pandas-dev/pandas/pull/32479#discussion_r399480077

@author:
There is now a different fix applied, a slice assignment to a new column will still create an `object` `dtype` for most extension array types.

diff_hunk: @@ -23,6 +23,7 @@ Fixed regressions
 - Fixed regression where :func:`read_pickle` raised a ``UnicodeDecodeError`` when reading a py27 pickle with :class:`MultiIndex` column (:issue:`31988`).
 - Fixed regression in :class:`DataFrame` arithmetic operations with mis-matched columns (:issue:`31623`)
 - Fixed regression in :meth:`GroupBy.agg` calling a user-provided function an extra time on an empty input (:issue:`31760`)
+- Fixed regression in :meth:`loc.__setitem__` throwing an error when a datetime64[ns, tz] value is provided (:issue:`32395`)
path: doc/source/whatsnew/v1.0.2.rst
text: @user1:
loc isn't a toplevel method. 

```suggestion
- Fixed regression in :meth:`DataFrame.loc` throwing an error when a ``datetime64[ns, tz]`` value is provided (:issue:`32395`)
```

@author:
The change is at the block level, it affects both `DataFrame.loc` and `Series.loc`, I'll update the file.