Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ Indexing
- Bug in :class:`Index` constructor where an unhelpful error message was raised for ``numpy`` scalars (:issue:`33017`)
- Bug in :meth:`DataFrame.lookup` incorrectly raising an ``AttributeError`` when ``frame.index`` or ``frame.columns`` is not unique; this will now raise a ``ValueError`` with a helpful error message (:issue:`33041`)
- Bug in :meth:`DataFrame.iloc.__setitem__` creating a new array instead of overwriting ``Categorical`` values in-place (:issue:`32831`)
- Bug in :meth:`DataFrame.copy` _item_cache invalidated after copy performs consolidation (:issue: `31784`)

Missing
^^^^^^^
Expand Down
1 change: 1 addition & 0 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -5680,6 +5680,7 @@ def copy(self: FrameOrSeries, deep: bool_t = True) -> FrameOrSeries:
dtype: object
"""
data = self._data.copy(deep=deep)
self._clear_item_cache()
return self._constructor(data).__finalize__(self)

def __copy__(self: FrameOrSeries, deep: bool_t = True) -> FrameOrSeries:
Expand Down
18 changes: 18 additions & 0 deletions pandas/tests/generic/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -910,3 +910,21 @@ def test_axis_classmethods(self, box):
assert obj._get_axis_number(v) == box._get_axis_number(v)
assert obj._get_axis_name(v) == box._get_axis_name(v)
assert obj._get_block_manager_axis(v) == box._get_block_manager_axis(v)

def test_cache_on_copy(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should go in tests/frame/methods/test_copy.py

can you make sure the stuff being done below is the minimal amount of stuff needed to trigger the bug in the status quo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked and it seems the minimal amount is being done to trigger the bug. The reason df["y"] = [0] is added is because it causes df["a"].values[0] to be incorrect in addition to df when the cache is not invalidated on copy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbrockmendel

this should go in tests/frame/methods/test_copy.py

yes we should do that but as a followon (to capture all _copy) tests.

df = DataFrame({"a": [1]})

df["x"] = [0]
df["a"]

df.copy()

df["a"].values[0] = -1

assert df["a"].values[0] == -1
tm.assert_frame_equal(df, DataFrame({"a": [-1], "x": [0]}))

df["y"] = 0

assert df["a"].values[0] == -1
tm.assert_frame_equal(df, DataFrame({"a": [-1], "x": [0], "y": [0]}))