BUG/API: prohibit dtype-changing IntervalArray.__setitem__

[jbrockmendel]

• closes BUG: IntervalArray.__setitem__ creates copies incorrectly #27147 Not quite
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

Still needs a dedicated test, but this is also a non-trivial API change, so I want to get the ball rolling on discussion. cc @jschendel

[WillAyd]

Is this linking back to the correct issue? The linkage there isn't immediately obvious to me (may be lack of understanding)

[jreback]

lgtm. cc @jschendel

[jbrockmendel]

Is this linking back to the correct issue? The linkage there isn't immediately obvious to me (may be lack of understanding)

#27147 is about the data backing an IntervalArray (a pair of Index objects) being swapped out under certain __setitem__ calls, which breaks existing views. The __setitem__ calls that cause this problem are exactly the ones that are dtype-changing, which this PR disallows.

[jschendel]

This should probably have a whatsnew entry since it's a breaking change.

[jschendel]

The needs_float_conversion name might be a bit misleading here because it's not triggered when setting arbitrary float values but rather just when np.nan is included, so we're not generically disallowing setting float values. Maybe a more accurate error message would be along the lines "Cannot set float NaN to integer-backed IntervalArray".

FWIW the behavior when setting float values without np.nan included is to truncate the float and just use the integer component, which is consistent with numpy's behavior:

In [1]: import pandas as pd; import numpy as np

In [2]: ia = pd.arrays.IntervalArray.from_breaks(range(5))

In [3]: ia[0] = pd.Interval(0.9, 1.1)

In [4]: ia
Out[4]: 
<IntervalArray>
[(0, 1], (1, 2], (2, 3], (3, 4]]
Length: 4, closed: right, dtype: interval[int64]

In [5]: a = np.arange(4)

In [6]: a[0] = 0.9

In [7]: a[1] = 1.1

In [8]: a
Out[8]: array([0, 1, 2, 3])

I'm not crazy about that behavior but I'm fine with it since it's consistent with numpy.

[jschendel]

Since we're raising here we can remove the future references that are no longer being used:

pandas/pandas/core/arrays/interval.py

Lines 554 to 555 in 23b6b93

	if needs_float_conversion:
	left = left.astype("float")

pandas/pandas/core/arrays/interval.py

Lines 560 to 561 in 23b6b93

	if needs_float_conversion:
	right = right.astype("float")

[jschendel]

I'm fine with prohibiting dtype changing here. Seems more consistent with existing behavior in pandas/numpy and makes the logic easier.

It looks like the existing needs_float_conversion logic is incomplete though and only handles the scalar case. Setting a slice with a list or IntervalArray containing np.nan doesn't raise or change dtype but instead takes the integer sentinel value (not sure if that's the right term?):

In [2]: ia = pd.arrays.IntervalArray.from_breaks(range(5))

In [3]: ia[:2] = [np.nan, pd.Interval(0, 5)]

In [4]: ia
Out[4]: 
<IntervalArray>
[(-9223372036854775808, -9223372036854775808], (0, 5], (2, 3], (3, 4]]
Length: 4, closed: right, dtype: interval[int64]

I think we can address this by shifting the logic around a little bit. The following diff addresses the issue locally for me and at first glance don't appear to break anything:

diff --git a/pandas/core/arrays/interval.py b/pandas/core/arrays/interval.py
index 22ce5a6f8..2e6e4bd0c 100644
--- a/pandas/core/arrays/interval.py
+++ b/pandas/core/arrays/interval.py
@@ -513,18 +513,15 @@ class IntervalArray(IntervalMixin, ExtensionArray):
         return self._shallow_copy(left, right)
 
     def __setitem__(self, key, value):
-        # na value: need special casing to set directly on numpy arrays
-        needs_float_conversion = False
         if is_scalar(value) and isna(value):
-            if is_integer_dtype(self.dtype.subtype):
-                # can't set NaN on a numpy integer array
-                needs_float_conversion = True
-            elif is_datetime64_any_dtype(self.dtype.subtype):
+            if is_datetime64_any_dtype(self.dtype.subtype):
                 # need proper NaT to set directly on the numpy array
                 value = np.datetime64("NaT")
             elif is_timedelta64_dtype(self.dtype.subtype):
                 # need proper NaT to set directly on the numpy array
                 value = np.timedelta64("NaT")
+            else:
+                value = np.nan
             value_left, value_right = value, value
 
         # scalar interval
@@ -542,18 +539,18 @@ class IntervalArray(IntervalMixin, ExtensionArray):
                 msg = f"'value' should be an interval type, got {type(value)} instead."
                 raise TypeError(msg) from err
 
+        if is_integer_dtype(self.dtype.subtype) and np.any(isna(value_left)):
+            raise ValueError("Cannot set float NaN to integer-backed IntervalArray")

[jreback]

@jbrockmendel can you rebase and does @jschendel comments make sense?

[jbrockmendel]

rebased+green; addressed some comments by @jschendel, commented above on reasons for punting on others

[jreback]

thanks @jbrockmendel and @jschendel