-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: Add lazy copy to replace #50746
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 15 commits
b855f09
e27d788
f638abd
bc3af5c
66b957f
4ecf604
2266ab8
7aee847
5f8fa06
6dc05e1
6514131
3849842
f2b8a97
c1d90ca
e8e1ff2
9e84de4
7988a16
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 |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
|
||
import pandas as pd | ||
from pandas import DataFrame | ||
import pandas._testing as tm | ||
from pandas.tests.copy_view.util import get_array | ||
|
||
|
||
|
@@ -93,3 +94,48 @@ def test_switch_options(): | |
subset.iloc[0, 0] = 0 | ||
# df updated with CoW disabled | ||
assert df.iloc[0, 0] == 0 | ||
|
||
|
||
@td.skip_array_manager_invalid_test | ||
@pytest.mark.parametrize( | ||
"locs, arr", | ||
[ | ||
([0], np.array([-1, -2, -3], dtype=np.intp)), | ||
([1], np.array([-1, -2, -3], dtype=np.intp)), | ||
([5], np.array([-1, -2, -3], dtype=np.intp)), | ||
([0, 1], np.array([-1, -2, -3], dtype=np.intp)), | ||
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. cc @lithomas1 I don't think that this is valid or can be reached in this way. I think the dimension of your values has to be the same as your indexer. This raises if blk.should_store_value is False, because it does not get broadcast. 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. You're right. Even for the non CoW, the iset operation doesn't fail(strangely), but printing the DataFrame afterwards does. 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. Yep you get a block that has dimension 2 according to block placement but the underlying array has only one column, doesn’t really error when creating but as soon as you access it youll get into trouble :) |
||
([0, 2], np.array([-1, -2, -3], dtype=np.intp)), | ||
([0, 1, 2], np.array([-1, -2, -3], dtype=np.intp)), | ||
([1, 2], np.array([-1, -2, -3], dtype=np.intp)), | ||
([1, 3], np.array([-1, -2, -3], dtype=np.intp)), | ||
([1, 3], np.array([[-1, -2, -3], [-4, -5, -6]], dtype=np.intp).T), | ||
], | ||
) | ||
def test_iset_splits_blocks_inplace(using_copy_on_write, locs, arr): | ||
# Nothing currently calls iset with | ||
# more than 1 loc with inplace=True (only happens with inplace=False) | ||
# but ensure that it works | ||
df = DataFrame( | ||
{ | ||
"a": [1, 2, 3], | ||
"b": [4, 5, 6], | ||
"c": [7, 8, 9], | ||
"d": [10, 11, 12], | ||
"e": [13, 14, 15], | ||
}, | ||
dtype=np.intp, | ||
) | ||
df["f"] = ["a", "b", "c"] | ||
df_orig = df.copy() | ||
df2 = df.copy(deep=None) # Trigger a CoW (if enabled, otherwise makes copy) | ||
df2._mgr.iset(locs, arr, inplace=True) | ||
|
||
tm.assert_frame_equal(df, df_orig) | ||
|
||
if using_copy_on_write: | ||
for i, col in enumerate(df.columns): | ||
if i not in locs: | ||
assert np.shares_memory(get_array(df, col), get_array(df2, col)) | ||
else: | ||
for col in df.columns: | ||
assert not np.shares_memory(get_array(df, col), get_array(df2, col)) |
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.
Do we need to clear the refs at
self.refs[blkno_l]
now, since first_nb should not have a reference block?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.
Thx for catching, must have removed the line accidentially
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.
Adjusted one of the tests to cover this