-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Add __copy__ and __deepcopy__ to NDFrame #15444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9721041
820664c
7e67e7d
1aea940
35f3e0f
d58b1f6
bf36f35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
# pylint: disable-msg=E1101,W0612 | ||
|
||
from operator import methodcaller | ||
from copy import copy, deepcopy | ||
import pytest | ||
import numpy as np | ||
from numpy import nan | ||
|
@@ -675,6 +676,17 @@ def test_validate_bool_args(self): | |
with self.assertRaises(ValueError): | ||
super(DataFrame, df).mask(cond=df.a > 2, inplace=value) | ||
|
||
def test_copy_and_deepcopy(self): | ||
for shape in [0, 1, 2]: | ||
obj = self._construct(shape) | ||
for func in [copy, | ||
deepcopy, | ||
lambda x: x.copy(deep=False), | ||
lambda x: x.copy(deep=True)]: | ||
obj_copy = func(obj) | ||
self.assertIsNot(obj_copy, obj) | ||
self._compare(obj_copy, obj) | ||
|
||
|
||
class TestSeries(tm.TestCase, Generic): | ||
_typ = Series | ||
|
@@ -1539,6 +1551,14 @@ def test_to_xarray(self): | |
expected, | ||
check_index_type=False) | ||
|
||
def test_deepcopy_empty(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. look at how the other tests are done. We need systematic testing on all structures. So move to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see my comment below on how test should be for this. you need to tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid I don't understand your comment here. Can you point me to a specific example to base the tests on? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I think I might not have made the motivation for this test sufficiently clear. It's not intended to be a full test of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bmcfee well you are changing fundamental things, which need full testing.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jreback sure, but that would be a separate test from the one you said to move. That issue never affected series; as far as I can tell, there's no way that it could have since it depends on having columns. I have no problem adding generic tests for copy/deepcopy, but I want to make sure it's the right test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bmcfee you can add the test you wanted. but you are making fundamental changes. These HAVE to be tested on all classes that are affected. (and you ARE affecting everything) |
||
# This test covers empty frame copying with non-empty column sets | ||
# as reported in issue GH15370 | ||
empty_frame = DataFrame(data=[], index=[], columns=['A']) | ||
empty_frame_copy = deepcopy(empty_frame) | ||
|
||
self._compare(empty_frame_copy, empty_frame) | ||
|
||
|
||
class TestPanel(tm.TestCase, Generic): | ||
_typ = Panel | ||
|
@@ -1569,6 +1589,9 @@ class TestPanel4D(tm.TestCase, Generic): | |
def test_sample(self): | ||
pytest.skip("sample on Panel4D") | ||
|
||
def test_copy_and_deepcopy(self): | ||
pytest.skip("copy_and_deepcopy on Panel4D") | ||
|
||
def test_to_xarray(self): | ||
|
||
tm._skip_if_no_xarray() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add (and test)
__deepcopy__
here as wellThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__deepcopy__
already exists in this class, see line 1483. Did you want me to move it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, then just move it next to this one