Repository: pandas-dev/pandas
Pull request: BUG: Fix inserting of wrong-dtyped NaT, closes #27297
- [x] closes #27297
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Needs tests for PeriodDType case.  The tests are probably not in the ideal place, how to move/parametrize them depends on what axis we want to sort them along (i.e. all Series indexing tests together or all timedelta-insertion tests together)

Problem statement:
BUG: DatetimeArray/Series incorrectly accepts timedelta64('NaT') for __setitem__
```
dti = pd.date_range('2016-01-01', periods=3)
nat = np.timedelta64('NaT')

dta = dti._data.copy()
dta[0] = nat  # <-- should raise TypeError, doesn't

ser = pd.Series(dti.copy())
ser[1] = nat  # <-- should raise TypeError, doesn't

df = pd.DataFrame(dti.copy())
df.loc[2, 0] = nat  # <-- should raise TypeError, doesn't
```

I haven't checked, but I expect the same thing happens in reverse for Timedelta, possible Period dtypes.

Hints from issue discussion:

Commit message: BUG: fix+test assignment of wrong nat to DataFrame

Patch to review:
diff --git a/pandas/_libs/index.pyx b/pandas/_libs/index.pyx
index ba2838d59f814..22d9bbb4e1307 100644
--- a/pandas/_libs/index.pyx
+++ b/pandas/_libs/index.pyx
@@ -534,7 +534,9 @@ cpdef convert_scalar(ndarray arr, object value):
         elif isinstance(value, (datetime, np.datetime64, date)):
             return Timestamp(value).value
         elif value is None or value != value:
-            return NPY_NAT
+            if not util.is_timedelta64_object(value):
+                # exclude np.timedelta64("NaT")
+                return NPY_NAT
         elif isinstance(value, str):
             return Timestamp(value).value
         raise ValueError("cannot set a Timestamp with a non-timestamp")
@@ -545,7 +547,9 @@ cpdef convert_scalar(ndarray arr, object value):
         elif isinstance(value, timedelta):
             return Timedelta(value).value
         elif value is None or value != value:
-            return NPY_NAT
+            if not util.is_datetime64_object(value):
+                # exclude np.datetime64("NaT")
+                return NPY_NAT
         elif isinstance(value, str):
             return Timedelta(value).value
         raise ValueError("cannot set a Timedelta with a non-timedelta")
diff --git a/pandas/core/arrays/datetimelike.py b/pandas/core/arrays/datetimelike.py
index 540442b7eaed4..a1d2f38268e1b 100644
--- a/pandas/core/arrays/datetimelike.py
+++ b/pandas/core/arrays/datetimelike.py
@@ -492,7 +492,7 @@ def __setitem__(
         elif isinstance(value, self._scalar_type):
             self._check_compatible_with(value)
             value = self._unbox_scalar(value)
-        elif isna(value) or value == iNaT:
+        elif is_valid_na(value, self.dtype) or value == iNaT:
             value = iNaT
         else:
             msg = (
@@ -1518,6 +1518,30 @@ def mean(self, skipna=True):
         return self._box_func(result)
 
 
+def is_valid_na(obj, dtype):
+    """
+    isna check that excludes incompatible dtypes
+
+    Parameters
+    ----------
+    obj : object
+    dtype : np.datetime64, np.timedelta64, DatetimeTZDtype, or PeriodDtype
+
+    Returns
+    -------
+    bool
+    """
+    if not isna(obj):
+        return False
+    if dtype.kind == "M":
+        return not isinstance(obj, np.timedelta64)
+    if dtype.kind == "m":
+        return not isinstance(obj, np.datetime64)
+
+    # must be PeriodDType
+    return not isinstance(obj, (np.datetime64, np.timedelta64))
+
+
 # -------------------------------------------------------------------
 # Shared Constructor Helpers
 
diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py
index bf6ebf1abe760..6c90a5f11a9bc 100644
--- a/pandas/core/internals/blocks.py
+++ b/pandas/core/internals/blocks.py
@@ -2295,7 +2295,8 @@ def _try_coerce_args(self, other):
         -------
         base-type other
         """
-        if is_null_datetimelike(other):
+        if is_null_datetimelike(other) and not isinstance(other, np.timedelta64):
+            # exclude np.timedelta64("NaT")
             other = tslibs.iNaT
         elif isinstance(other, (datetime, np.datetime64, date)):
             other = self._box_func(other)
@@ -2485,7 +2486,8 @@ def _try_coerce_args(self, other):
             # add the tz back
             other = self._holder(other, dtype=self.dtype)
 
-        elif is_null_datetimelike(other):
+        if is_null_datetimelike(other) and not isinstance(other, np.timedelta64):
+            # exclude np.timedelta64("NaT")
             other = tslibs.iNaT
         elif isinstance(other, self._holder):
             if other.tz != self.values.tz:
@@ -2589,8 +2591,11 @@ def setitem(self, indexer, value):
         try:
             return super().setitem(indexer, value)
         except (ValueError, TypeError):
+            obj_vals = self.values.astype(object)
+            if self.ndim == 2 and obj_vals.ndim == 1:
+                obj_vals = obj_vals.reshape(1, -1)
             newb = make_block(
-                self.values.astype(object), placement=self.mgr_locs, klass=ObjectBlock
+                obj_vals, placement=self.mgr_locs, klass=ObjectBlock, ndim=self.ndim
             )
             return newb.setitem(indexer, value)
 
@@ -2665,7 +2670,8 @@ def _try_coerce_args(self, other):
         base-type other
         """
 
-        if is_null_datetimelike(other):
+        if is_null_datetimelike(other) and not isinstance(other, np.datetime64):
+            # exclude np.datetime64("NaT")
             other = tslibs.iNaT
         elif isinstance(other, (timedelta, np.timedelta64)):
             other = Timedelta(other).value
diff --git a/pandas/core/series.py b/pandas/core/series.py
index b3a7f38aef8ef..356dfe4b6dcb4 100644
--- a/pandas/core/series.py
+++ b/pandas/core/series.py
@@ -1210,7 +1210,8 @@ def setitem(key, value):
                     pass
                 elif is_timedelta64_dtype(self.dtype):
                     # reassign a null value to iNaT
-                    if isna(value):
+                    if isna(value) and not isinstance(value, np.datetime64):
+                        # exclude np.datetime64("NaT")
                         value = iNaT
 
                         try:
diff --git a/pandas/tests/series/indexing/test_indexing.py b/pandas/tests/series/indexing/test_indexing.py
index c8342c54e9b5d..60188f3dfc8c7 100644
--- a/pandas/tests/series/indexing/test_indexing.py
+++ b/pandas/tests/series/indexing/test_indexing.py
@@ -654,6 +654,145 @@ def test_timedelta_assignment():
     tm.assert_series_equal(s, expected)
 
 
+def test_timedelta_nat_assignment_series():
+    tdi = pd.timedelta_range("1s", periods=3, freq="s")
+    base = pd.Series(tdi)
+    expected = pd.Series([pd.NaT, tdi[1], tdi[2]], dtype="m8[ns]")
+
+    casting_nas = [pd.NaT, np.timedelta64("NaT", "ns")]
+    for nat in casting_nas:
+        ser = base.copy(deep=True)
+        ser[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        ser = base.copy(deep=True)
+        ser.loc[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        df = base.copy(deep=True).to_frame()
+        df.loc[0, 0] = nat
+        tm.assert_frame_equal(df, expected.to_frame())
+
+        df = base.copy(deep=True).to_frame()
+        df.iloc[0, 0] = nat
+        tm.assert_frame_equal(df, expected.to_frame())
+
+    # a specifically-datetime NaT should not be coerced to timedelta
+    expected = expected.astype(object)
+    non_casting_nas = [np.datetime64("NaT", "ns")]
+    for nat in non_casting_nas:
+        ser = base.copy(deep=True)
+        ser[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        ser = base.copy(deep=True)
+        ser.loc[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        ser = base.copy(deep=True)
+        ser.iloc[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        df = base.copy(deep=True).to_frame()
+        df.loc[0, 0] = nat
+        tm.assert_frame_equal(df, pd.DataFrame(expected, dtype=object))
+
+        df = base.copy(deep=True).to_frame()
+        df.iloc[0, 0] = nat
+        tm.assert_frame_equal(df, pd.DataFrame(expected, dtype=object))
+
+
+@pytest.mark.parametrize("tz", [None, "US/Pacific"])
+def test_datetime_nat_assignment_series(tz):
+    dti = pd.date_range("2016-01-01", periods=3, tz=tz)
+    base = pd.Series(dti)
+    expected = pd.Series([pd.NaT, dti[1], dti[2]], dtype=dti.dtype)
+
+    casting_nas = [pd.NaT, np.datetime64("NaT", "ns")]
+    for nat in casting_nas:
+        ser = base.copy(deep=True)
+        ser[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        ser = base.copy(deep=True)
+        ser.loc[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        df = base.copy(deep=True).to_frame()
+        df.loc[0, 0] = nat
+        tm.assert_frame_equal(df, expected.to_frame())
+
+        df = base.copy(deep=True).to_frame()
+        df.iloc[0, 0] = nat
+        tm.assert_frame_equal(df, expected.to_frame())
+
+    # a specifically-timedelta NaT should not be coerced to datetime
+    expected = expected.astype(object)
+    non_casting_nas = [np.timedelta64("NaT", "ns")]
+    for nat in non_casting_nas:
+        expected[0] = nat
+        assert expected[0] is nat
+
+        ser = base.copy(deep=True)
+        ser[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        ser = base.copy(deep=True)
+        ser.loc[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        ser = base.copy(deep=True)
+        ser.iloc[0] = nat
+        tm.assert_series_equal(ser, expected)
+
+        df = base.copy(deep=True).to_frame()
+        df.loc[0, 0] = nat
+        tm.assert_frame_equal(df, pd.DataFrame(expected, dtype=object))
+
+        df = base.copy(deep=True).to_frame()
+        df.iloc[0, 0] = nat
+        tm.assert_frame_equal(df, pd.DataFrame(expected, dtype=object))
+
+
+def test_timedelta_nat_assignment_array():
+    tdi = pd.timedelta_range("1s", periods=3, freq="s")
+    tda = tdi._data
+    expected = pd.TimedeltaIndex([pd.NaT, tdi[1], tdi[2]])._data
+
+    casting_nas = [pd.NaT, np.timedelta64("NaT", "ns")]
+    for nat in casting_nas:
+        arr = tda.copy()
+        arr[0] = nat
+
+        tm.assert_equal(arr, expected)
+
+    non_casting_nas = [np.datetime64("NaT", "ns")]
+    for nat in non_casting_nas:
+        arr = tda.copy()
+        with pytest.raises(TypeError):
+            arr[0] = nat
+
+
+@pytest.mark.parametrize("tz", [None, "US/Pacific"])
+def test_datetime_nat_assignment_array(tz):
+    dti = pd.date_range("2016-01-01", periods=3, tz=tz)
+    dta = dti._data
+    expected = pd.DatetimeIndex([pd.NaT, dti[1], dti[2]])._data
+
+    casting_nas = [pd.NaT, np.datetime64("NaT", "ns")]
+    for nat in casting_nas:
+        arr = dta.copy()
+        arr[0] = nat
+
+        tm.assert_equal(arr, expected)
+
+    non_casting_nas = [np.timedelta64("NaT", "ns")]
+    for nat in non_casting_nas:
+        arr = dta.copy()
+        with pytest.raises(TypeError):
+            arr[0] = nat
+
+
 def test_underlying_data_conversion():
     # GH 4080
     df = DataFrame({c: [1, 2, 3] for c in ["a", "b", "c"]})


Code review comments:
diff_hunk: @@ -1518,6 +1518,30 @@ def mean(self, skipna=True):
         return self._box_func(result)
 
 
+def is_valid_na(obj, dtype):
path: pandas/core/arrays/datetimelike.py
text: @user1:
what??? yet another missing value check. This should not be here at all.

@author:
i guess it could go in _libs.missing?

diff_hunk: @@ -534,7 +534,9 @@ cpdef convert_scalar(ndarray arr, object value):
         elif isinstance(value, (datetime, np.datetime64, date)):
             return Timestamp(value).value
         elif value is None or value != value:
-            return NPY_NAT
+            if not util.is_timedelta64_object(value):
path: pandas/_libs/index.pyx
text: @user1:
can we simply check is_timedelta64_object at a higher level (e.g. the if/elif level) to avoid the nested if calls (here and below), and simply dispatch on it directly

@author:
sure