Skip to content

Conversation

@neilkg
Copy link
Contributor

@neilkg neilkg commented Apr 5, 2020

@neilkg neilkg changed the title Clear cache on copy BUG: DataFrame._item_cache not cleared on on .copy() Apr 5, 2020
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.

@neilkg neilkg requested a review from jreback April 5, 2020 16:48
@jreback jreback added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode Indexing Related to indexing on series/frames, not to indexes themselves labels Apr 5, 2020


class TestCopy:
def test_cache_on_copy(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

put this in pandas/tests/frame/methods/test_api with the rest of the copy tests.

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 only see other copy tests in pandas/tests/frame/test_api.py. I'll assume you mean that one.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes



class TestCopy:
def test_cache_on_copy(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number as a comment as well as a 1-line expl

- 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`)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you elaborate on what the user affects are here.

@neilkg neilkg requested a review from jreback April 5, 2020 22:16
@jreback jreback added this to the 1.1 milestone Apr 6, 2020
@jreback
Copy link
Contributor

jreback commented Apr 6, 2020

lgtm. merging pending doc-build merge (so nothing to do @neilkg )

@jreback jreback merged commit 4fc1758 into pandas-dev:master Apr 6, 2020
@jreback
Copy link
Contributor

jreback commented Apr 6, 2020

thanks @neilkg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Indexing Related to indexing on series/frames, not to indexes themselves Reshaping Concat, Merge/Join, Stack/Unstack, Explode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: DataFrame._item_cache not cleared on on .copy()

4 participants