BUG: df[col] = arr should not overwrite data in df[col]

doc/source/whatsnew/v1.3.0.rst

@@ -31,7 +31,82 @@ Notable bug fixes
 
 These are bug fixes that might have notable behavior changes.
 
+Assigning with ``DataFrame.__setitem__`` consistently creates a new array
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+Assigning values with ``DataFrame.__setitem__`` now consistently assigns a new array, rather than mutating inplace (:issue:`33457`, :issue:`35271`, :issue:`35266`)
+
+Previously, ``DataFrame.__setitem__`` would sometimes operate inplace on the
+underlying array, and sometimes assign a new array. Fixing this inconsistency
+can have behavior-changing implications for workloads that relied on inplace
+mutation. The two most common cases are creating a ``DataFrame`` from an array
+and slicing a ``DataFrame``.
+
+*Previous Behavior*
+
+The array would be mutated inplace for some dtypes, like NumPy's ``int64`` dtype.
+
+.. code-block:: python
+
+   >>> import pandas as pd
+   >>> import numpy as np
+   >>> a = np.array([1, 2, 3])
+   >>> df = pd.DataFrame(a, columns=['a'])
+   >>> df['a'] = 0
+   >>> a  # mutated inplace
+   array([0, 0, 0])
+
+But not others, like :class:`Int64Dtype`.
+
+.. code-block:: python
+
+   >>> import pandas as pd
+   >>> import numpy as np
+   >>> a = pd.array([1, 2, 3], dtype="Int64")
+   >>> df = pd.DataFrame(a, columns=['a'])
+   >>> df['a'] = 0
+   >>> a  # not mutated
+   <IntegerArray>
+   [1, 2, 3]
+   Length: 3, dtype: Int64
+
+
+*New Behavior*
+
+In pandas 1.3.0, ``DataFrame.__setitem__`` consistently sets on a new array rather than
+mutating the existing array inplace.
+
+For NumPy's int64 dtype
+
+.. ipython:: python
+
+   import pandas as pd
+   import numpy as np
+   a = np.array([1, 2, 3])
+   df = pd.DataFrame(a, columns=['a'])
+   df['a'] = 0
+   a  # not mutated
+
+For :class:`Int64Dtype`.
+
+.. ipython:: python
+
+   import pandas as pd
+   import numpy as np
+   a = pd.array([1, 2, 3], dtype="Int64")
+   df = pd.DataFrame(a, columns=['a'])
+   df['a'] = 0
+   a  # not mutated
+
+This also affects cases where a second ``Series`` or ``DataFrame`` is a view on a first ``DataFrame``.
+
+.. code-block:: python
+
+   df = pd.DataFrame({"A": [1, 2, 3]})
+   df2 = df[['A']]
+   df['A'] = np.array([0, 0, 0])
+
+Previously, whether ``df2`` was mutated depending on the dtype of the array being assigned to. Now, a new array is consistently assigned, so ``df2`` is not mutated.
 
 .. _whatsnew_130.api_breaking.deps:
 
@@ -127,7 +202,7 @@ Interval
 
 Indexing
 ^^^^^^^^
-
+- Bug in :meth:`DataFrame.iloc.__setitem__` creating a new array instead of overwriting ``Categorical`` values in-place (:issue:`35417`)
 -
 -
 

pandas/_libs/reduction.pyx

@@ -406,7 +406,8 @@ cdef class BlockSlider:
     """
     cdef:
         object frame, dummy, index, block
-        list blk_values
+        list blocks, blk_values
+        ndarray orig_blklocs, orig_blknos
         ndarray values
         Slider idx_slider
         char **base_ptrs
@@ -418,6 +419,13 @@ cdef class BlockSlider:
         self.dummy = frame[:0]
         self.index = self.dummy.index
 
+        # GH#35417 attributes we need to restore at each step in case
+        #  the function modified them.
+        mgr = self.dummy._mgr
+        self.orig_blklocs = mgr.blklocs
+        self.orig_blknos = mgr.blknos
+        self.blocks = [x for x in self.dummy._mgr.blocks]
+
         self.blk_values = [block.values for block in self.dummy._mgr.blocks]
 
         for values in self.blk_values:
@@ -441,6 +449,9 @@ cdef class BlockSlider:
         cdef:
             ndarray arr
             Py_ssize_t i
+
+        self._restore_blocks()
+
         # move blocks
         for i in range(self.nblocks):
             arr = self.blk_values[i]
@@ -460,9 +471,21 @@ cdef class BlockSlider:
         cdef:
             ndarray arr
             Py_ssize_t i
+
+        self._restore_blocks()
+
         for i in range(self.nblocks):
             arr = self.blk_values[i]
 
             # axis=1 is the frame's axis=0
             arr.data = self.base_ptrs[i]
             arr.shape[1] = 0
+
+    cdef _restore_blocks(self):
+        """
+        Ensure that we have the original blocks, blknos, and blklocs.
+        """
+        mgr = self.dummy._mgr
+        mgr.blocks = self.blocks
+        mgr._blklocs = self.orig_blklocs
+        mgr._blknos = self.orig_blknos

pandas/_testing.py

@@ -1140,6 +1140,8 @@ def _get_base(obj):
             raise AssertionError(f"{repr(left_base)} is {repr(right_base)}")
 
     def _raise(left, right, err_msg):
+        __tracebackhide__ = True
+
         if err_msg is None:
             if left.shape != right.shape:
                 raise_assert_detail(

pandas/core/frame.py

@@ -3211,17 +3211,17 @@ def _setitem_frame(self, key, value):
         self._check_setitem_copy()
         self._where(-key, value, inplace=True)
 
-    def _iset_item(self, loc: int, value):
+    def _iset_item(self, loc: int, value, inplace: bool = False):
 
         # technically _sanitize_column expects a label, not a position,
         #  but the behavior is the same as long as we pass broadcast=False
         value = self._sanitize_column(loc, value, broadcast=False)
-        NDFrame._iset_item(self, loc, value)
+        NDFrame._iset_item(self, loc, value, inplace=inplace)
 
         # check if we are modifying a copy
         # try to set first as we want an invalid
         # value exception to occur first
-        if len(self):
+        if len(self):  # FIXME: this should depend on inplace, right?
             self._check_setitem_copy()
 
     def _set_item(self, key, value):

pandas/core/generic.py

@@ -3425,7 +3425,7 @@ def _maybe_cache_changed(self, item, value) -> None:
         The object has called back to us saying maybe it has changed.
         """
         loc = self._info_axis.get_loc(item)
-        self._mgr.iset(loc, value)
+        self._mgr.iset(loc, value, inplace=False)
 
     @final
     @property
@@ -3814,8 +3814,8 @@ def _slice(self: FrameOrSeries, slobj: slice, axis=0) -> FrameOrSeries:
         result._set_is_copy(self, copy=is_copy)
         return result
 
-    def _iset_item(self, loc: int, value) -> None:
-        self._mgr.iset(loc, value)
+    def _iset_item(self, loc: int, value, inplace: bool_t = False) -> None:
+        self._mgr.iset(loc, value, inplace=inplace)
         self._clear_item_cache()
 
     def _set_item(self, key, value) -> None:

pandas/core/indexing.py

@@ -11,6 +11,7 @@
 from pandas.errors import AbstractMethodError, InvalidIndexError
 from pandas.util._decorators import doc
 
+from pandas.core.dtypes.cast import maybe_infer_dtype_type
 from pandas.core.dtypes.common import (
     is_array_like,
     is_hashable,
@@ -675,8 +676,22 @@ def _ensure_listlike_indexer(self, key, axis=None, value=None):
             # GH#38148
             keys = self.obj.columns.union(key, sort=False)
 
+            # Try to get the right dtype when we do this reindex.
+            fv = None
+            if is_scalar(value):
+                fv = value
+            else:
+                dtype = maybe_infer_dtype_type(value)
+                if dtype is not None:
+                    fv = dtype.type(0)  # TODO: or try to just do value[0]?
+
             self.obj._mgr = self.obj._mgr.reindex_axis(
-                keys, axis=0, copy=False, consolidate=False, only_slice=True
+                keys,
+                axis=0,
+                copy=False,
+                consolidate=False,
+                only_slice=True,
+                fill_value=fv,
             )
 
     def __setitem__(self, key, value):
@@ -1576,7 +1591,6 @@ def _setitem_with_indexer(self, indexer, value, name="iloc"):
                     # essentially this separates out the block that is needed
                     # to possibly be modified
                     if self.ndim > 1 and i == info_axis:
-
                         # add the new item, and set the value
                         # must have all defined axes if we have a scalar
                         # or a list-like on the non-info axes if we have a
@@ -1805,6 +1819,13 @@ def _setitem_single_column(self, loc: int, value, plane_indexer):
         # multi-dim object
         # GH#6149 (null slice), GH#10408 (full bounds)
         if com.is_null_slice(pi) or com.is_full_slice(pi, len(self.obj)):
+            blk = ser._mgr.blocks[0]
+            if blk._can_hold_element(value) and is_scalar(value):
+                # FIXME: ExtensionBlock._can_hold_element
+                # We can do an inplace-setting, do it directly on _values
+                #  to get our underlying
+                ser._values[plane_indexer] = value
+                return
             ser = value
         else:
             # set the item, possibly having a dtype change
@@ -1813,7 +1834,7 @@ def _setitem_single_column(self, loc: int, value, plane_indexer):
             ser._maybe_update_cacher(clear=True)
 
         # reset the sliced object if unique
-        self.obj._iset_item(loc, ser)
+        self.obj._iset_item(loc, ser, inplace=True)
 
     def _setitem_single_block(self, indexer, value, name: str):
         """
@@ -1839,7 +1860,9 @@ def _setitem_single_block(self, indexer, value, name: str):
                 )
                 and item_labels.is_unique
             ):
-                self.obj[item_labels[indexer[info_axis]]] = value
+                col = item_labels[indexer[info_axis]]
+                loc = item_labels.get_loc(col)
+                self.obj._iset_item(loc, value, inplace=True)
                 return
 
             indexer = maybe_convert_ix(*indexer)

pandas/core/internals/blocks.py

@@ -1724,7 +1724,7 @@ def set_inplace(self, locs, values):
         # NB: This is a misnomer, is supposed to be inplace but is not,
         #  see GH#33457
         assert locs.tolist() == [0]
-        self.values = values
+        self.values[:] = values
 
     def putmask(
         self, mask, new, inplace: bool = False, axis: int = 0, transpose: bool = False

pandas/core/internals/managers.py

@@ -1054,7 +1054,7 @@ def idelete(self, indexer):
         )
         self._rebuild_blknos_and_blklocs()
 
-    def iset(self, loc: Union[int, slice, np.ndarray], value):
+    def iset(self, loc: Union[int, slice, np.ndarray], value, inplace: bool = False):
         """
         Set new item in-place. Does not consolidate. Adds new Block if not
         contained in the current set of items
@@ -1106,7 +1106,7 @@ def value_getitem(placement):
         for blkno, val_locs in libinternals.get_blkno_placements(blknos, group=True):
             blk = self.blocks[blkno]
             blk_locs = blklocs[val_locs.indexer]
-            if blk.should_store(value):
+            if inplace and blk.should_store(value):
                 blk.set_inplace(blk_locs, value_getitem(val_locs))
             else:
                 unfit_mgr_locs.append(blk.mgr_locs.as_array[blk_locs])

pandas/tests/frame/indexing/test_indexing.py

@@ -168,6 +168,7 @@ def test_setitem_list_of_tuples(self, float_frame):
                 DataFrame(
                     [[1, 2, 7, 8], [3, 4, 7, 8], [5, 6, 7, 8]],
                     columns=["A", "B", "C", "D"],
+                    dtype=np.intp,
                 ),
             ),
             (
@@ -809,7 +810,7 @@ def test_fancy_getitem_slice_mixed(self, float_frame, float_string_frame):
 
         msg = r"\nA value is trying to be set on a copy of a slice from a DataFrame"
         with pytest.raises(com.SettingWithCopyError, match=msg):
-            sliced["C"] = 4.0
+            sliced.loc[:, "C"] = 4.0
 
         assert (float_frame["C"] == 4).all()
 
@@ -1286,7 +1287,7 @@ def test_iloc_row(self):
         # setting it makes it raise/warn
         msg = r"\nA value is trying to be set on a copy of a slice from a DataFrame"
         with pytest.raises(com.SettingWithCopyError, match=msg):
-            result[2] = 0.0
+            result.loc[:, 2] = 0.0
 
         exp_col = df[2].copy()
         exp_col[4:8] = 0.0
@@ -1318,7 +1319,7 @@ def test_iloc_col(self):
         # and that we are setting a copy
         msg = r"\nA value is trying to be set on a copy of a slice from a DataFrame"
         with pytest.raises(com.SettingWithCopyError, match=msg):
-            result[8] = 0.0
+            result.loc[:, 8] = 0.0
 
         assert (df[8] == 0).all()
 

pandas/tests/frame/methods/test_rename.py

@@ -164,7 +164,7 @@ def test_rename_multiindex(self):
 
     def test_rename_nocopy(self, float_frame):
         renamed = float_frame.rename(columns={"C": "foo"}, copy=False)
-        renamed["foo"] = 1.0
+        renamed["foo"][:] = 1.0
         assert (float_frame["C"] == 1.0).all()
 
     def test_rename_inplace(self, float_frame):

pandas/tests/indexing/test_iloc.py

@@ -704,7 +704,7 @@ def test_identity_slice_returns_new_object(self):
         assert sliced_df is not original_df
 
         # should be a shallow copy
-        original_df["a"] = [4, 4, 4]
+        original_df.loc[:, "a"] = [4, 4, 4]
         assert (sliced_df["a"] == 4).all()
 
         original_series = Series([1, 2, 3, 4, 5, 6])
@@ -728,8 +728,8 @@ def test_series_indexing_zerodim_np_array(self):
         result = s.iloc[np.array(0)]
         assert result == 1
 
-    @pytest.mark.xfail(reason="https://github.com/pandas-dev/pandas/issues/33457")
     def test_iloc_setitem_categorical_updates_inplace(self):
+        # GH#35417
         # Mixed dtype ensures we go through take_split_path in setitem_with_indexer
         cat = Categorical(["A", "B", "C"])
         df = DataFrame({1: cat, 2: [1, 2, 3]})

pandas/tests/indexing/test_loc.py

@@ -902,7 +902,7 @@ def test_identity_slice_returns_new_object(self):
         assert original_df[:] is not original_df
 
         # should be a shallow copy
-        original_df["a"] = [4, 4, 4]
+        original_df["a"][:] = [4, 4, 4]
         assert (sliced_df["a"] == 4).all()
 
         # These should not return copies