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

doc/source/whatsnew/v1.1.0.rst

@@ -351,6 +351,83 @@ 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.1.0, ``DataFrame.__setitem__`` consistently sets on a new array rather than
+mutating the existing array inplace.
+
+.. ipython:: python
+
+For NumPy's int64 dtype
+
+   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`.
+
+   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.
+
 ``MultiIndex.get_indexer`` interprets ``method`` argument correctly
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

pandas/_testing.py

@@ -1089,6 +1089,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/internals/blocks.py

@@ -634,20 +634,6 @@ def _can_hold_element(self, element: Any) -> bool:
             return issubclass(tipo.type, dtype)
         return isinstance(element, dtype)
 
-    def should_store(self, value: ArrayLike) -> bool:
-        """
-        Should we set self.values[indexer] = value inplace or do we need to cast?
-
-        Parameters
-        ----------
-        value : np.ndarray or ExtensionArray
-
-        Returns
-        -------
-        bool
-        """
-        return is_dtype_equal(value.dtype, self.dtype)
-
     def to_native_types(self, na_rep="nan", quoting=None, **kwargs):
         """ convert to our native types format """
         values = self.values
@@ -1581,12 +1567,6 @@ def iget(self, col):
                 raise IndexError(f"{self} only contains one item")
             return self.values
 
-    def should_store(self, value: ArrayLike) -> bool:
-        """
-        Can we set the given array-like value inplace?
-        """
-        return isinstance(value, self._holder)
-
     def set(self, locs, values):
         assert locs.tolist() == [0]
         self.values[:] = values
@@ -1976,9 +1956,6 @@ def _can_hold_element(self, element: Any) -> bool:
             element, (float, int, complex, np.float_, np.int_)
         ) and not isinstance(element, (bool, np.bool_))
 
-    def should_store(self, value: ArrayLike) -> bool:
-        return issubclass(value.dtype.type, np.complexfloating)
-
 
 class IntBlock(NumericBlock):
     __slots__ = ()
@@ -2142,7 +2119,6 @@ class DatetimeTZBlock(ExtensionBlock, DatetimeBlock):
     _can_hold_element = DatetimeBlock._can_hold_element
     to_native_types = DatetimeBlock.to_native_types
     fill_value = np.datetime64("NaT", "ns")
-    should_store = Block.should_store
     array_values = ExtensionBlock.array_values
 
     @property
@@ -2616,8 +2592,6 @@ class CategoricalBlock(ExtensionBlock):
     _verify_integrity = True
     _can_hold_na = True
 
-    should_store = Block.should_store
-
     def __init__(self, values, placement, ndim=None):
         # coerce to categorical if we can
         values = extract_array(values)

pandas/core/internals/managers.py

@@ -1062,18 +1062,16 @@ 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):
-                blk.set(blk_locs, value_getitem(val_locs))
-            else:
-                unfit_mgr_locs.append(blk.mgr_locs.as_array[blk_locs])
-                unfit_val_locs.append(val_locs)
 
-                # If all block items are unfit, schedule the block for removal.
-                if len(val_locs) == len(blk.mgr_locs):
-                    removed_blknos.append(blkno)
-                else:
-                    blk.delete(blk_locs)
-                    self._blklocs[blk.mgr_locs.indexer] = np.arange(len(blk))
+            unfit_mgr_locs.append(blk.mgr_locs.as_array[blk_locs])
+            unfit_val_locs.append(val_locs)
+
+            # If all block items are unfit, schedule the block for removal.
+            if len(val_locs) == len(blk.mgr_locs):
+                removed_blknos.append(blkno)
+            else:
+                blk.delete(blk_locs)
+                self._blklocs[blk.mgr_locs.indexer] = np.arange(len(blk))
 
         if len(removed_blknos):
             # Remove blocks & update blknos accordingly

pandas/tests/frame/methods/test_rename.py

@@ -140,7 +140,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/frame/test_block_internals.py

@@ -322,7 +322,7 @@ def test_copy_blocks(self, float_frame):
         blocks = df._to_dict_of_blocks(copy=True)
         for dtype, _df in blocks.items():
             if column in _df:
-                _df.loc[:, column] = _df[column] + 1
+                _df.loc[:, column].values[:] = _df[column] + 1
 
         # make sure we did not change the original DataFrame
         assert not _df[column].equals(df[column])
@@ -334,12 +334,14 @@ def test_no_copy_blocks(self, float_frame):
 
         # use the copy=False, change a column
         blocks = df._to_dict_of_blocks(copy=False)
-        for dtype, _df in blocks.items():
+        for _, _df in blocks.items():
             if column in _df:
-                _df.loc[:, column] = _df[column] + 1
+                _df.loc[:, column].values[:] = _df[column] + 1
+                # FIXME: I think the need for .values here means we are doing something wrong
 
         # make sure we did change the original DataFrame
-        assert _df[column].equals(df[column])
+        tm.assert_series_equal(df[column], _df[column])
+        #assert _df[column].equals(df[column])
 
     def test_copy(self, float_frame, float_string_frame):
         cop = float_frame.copy()

pandas/tests/groupby/test_apply.py

@@ -966,27 +966,6 @@ def test_apply_function_index_return(function):
     tm.assert_series_equal(result, expected)
 
 
-def test_apply_function_with_indexing():
-    # GH: 33058
-    df = pd.DataFrame(
-        {"col1": ["A", "A", "A", "B", "B", "B"], "col2": [1, 2, 3, 4, 5, 6]}
-    )
-
-    def fn(x):
-        x.col2[x.index[-1]] = 0
-        return x.col2
-
-    result = df.groupby(["col1"], as_index=False).apply(fn)
-    expected = pd.Series(
-        [1, 2, 0, 4, 5, 0],
-        index=pd.MultiIndex.from_tuples(
-            [(0, 0), (0, 1), (0, 2), (1, 3), (1, 4), (1, 5)]
-        ),
-        name="col2",
-    )
-    tm.assert_series_equal(result, expected)
-
-
 def test_apply_function_with_indexing_return_column():
     # GH: 7002
     df = DataFrame(

pandas/tests/indexing/test_loc.py

@@ -877,7 +877,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

pandas/tests/internals/test_internals.py

@@ -642,33 +642,31 @@ def test_get_numeric_data(self):
             item_shape=(3,),
         )
         mgr.iset(5, np.array([1, 2, 3], dtype=np.object_))
-
         numeric = mgr.get_numeric_data()
         tm.assert_index_equal(
             numeric.items, pd.Index(["int", "float", "complex", "bool"])
         )
+        mgr_idx = mgr.items.get_loc("float")
+        num_idx = numeric.items.get_loc("float")
+
         tm.assert_almost_equal(
-            mgr.iget(mgr.items.get_loc("float")).internal_values(),
-            numeric.iget(numeric.items.get_loc("float")).internal_values(),
+            mgr.iget(mgr_idx).internal_values(),
+            numeric.iget(num_idx).internal_values(),
         )
 
         # Check sharing
-        numeric.iset(numeric.items.get_loc("float"), np.array([100.0, 200.0, 300.0]))
+        numeric.iget(num_idx).internal_values()[:] = [100.0, 200.0, 300.0]
         tm.assert_almost_equal(
-            mgr.iget(mgr.items.get_loc("float")).internal_values(),
-            np.array([100.0, 200.0, 300.0]),
+            mgr.iget(mgr_idx).internal_values(), np.array([100.0, 200.0, 300.0]),
         )
 
         numeric2 = mgr.get_numeric_data(copy=True)
         tm.assert_index_equal(
             numeric.items, pd.Index(["int", "float", "complex", "bool"])
         )
-        numeric2.iset(
-            numeric2.items.get_loc("float"), np.array([1000.0, 2000.0, 3000.0])
-        )
+        numeric2.iget(num_idx).internal_values()[:] = [1000.0, 2000.0, 3000.0]
         tm.assert_almost_equal(
-            mgr.iget(mgr.items.get_loc("float")).internal_values(),
-            np.array([100.0, 200.0, 300.0]),
+            mgr.iget(mgr_idx).internal_values(), np.array([100.0, 200.0, 300.0]),
         )
 
     def test_get_bool_data(self):
@@ -686,18 +684,20 @@ def test_get_bool_data(self):
             bools.iget(bools.items.get_loc("bool")).internal_values(),
         )
 
+        # GH#33457 setting a new array on bools does _not_ alter the original
+        #  array in-place, so mgr is unchanged
         bools.iset(0, np.array([True, False, True]))
         tm.assert_numpy_array_equal(
             mgr.iget(mgr.items.get_loc("bool")).internal_values(),
-            np.array([True, False, True]),
+            np.array([True, True, True]),
         )
 
-        # Check sharing
+        # Check non-sharing
         bools2 = mgr.get_bool_data(copy=True)
-        bools2.iset(0, np.array([False, True, False]))
+        bools2.blocks[0].values[:] = [[False, True, False]]
         tm.assert_numpy_array_equal(
             mgr.iget(mgr.items.get_loc("bool")).internal_values(),
-            np.array([True, False, True]),
+            np.array([True, True, True]),
         )
 
     def test_unicode_repr_doesnt_raise(self):
@@ -1181,23 +1181,6 @@ def test_binop_other(self, op, value, dtype):
             tm.assert_series_equal(result, expected)
 
 
-class TestShouldStore:
-    def test_should_store_categorical(self):
-        cat = pd.Categorical(["A", "B", "C"])
-        df = pd.DataFrame(cat)
-        blk = df._mgr.blocks[0]
-
-        # matching dtype
-        assert blk.should_store(cat)
-        assert blk.should_store(cat[:-1])
-
-        # different dtype
-        assert not blk.should_store(cat.as_ordered())
-
-        # ndarray instead of Categorical
-        assert not blk.should_store(np.asarray(cat))
-
-
 @pytest.mark.parametrize(
     "typestr, holder",
     [

pandas/tests/reshape/merge/test_merge.py

@@ -284,7 +284,7 @@ def test_merge_nocopy(self):
 
         merged = merge(left, right, left_index=True, right_index=True, copy=False)
 
-        merged["a"] = 6
+        merged.loc[:, "a"] = 6
         assert (left["a"] == 6).all()
 
         merged["d"] = "peekaboo"