-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: Use lazy copy in infer objects #50428
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 27 commits
3c4cee3
db5dba1
9eb4d85
081dd29
76c443b
f7724ff
a7b4e27
1d4f726
018cfe6
1696d8a
1782fbd
8cb6355
cead228
47d85b3
a3d0a2b
2e2ed0f
3a84382
64d550b
716cef8
f693829
97fa214
c3e4e66
5d687dd
f5bf65f
af8af95
f47f6dd
e357b64
bf1bb3b
5624983
9a6d516
8b0e2b0
3f832ba
9f11f0a
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 |
---|---|---|
|
@@ -207,7 +207,9 @@ def mgr_locs(self, new_mgr_locs: BlockPlacement) -> None: | |
self._mgr_locs = new_mgr_locs | ||
|
||
@final | ||
def make_block(self, values, placement=None) -> Block: | ||
def make_block( | ||
self, values, placement=None, refs: BlockValuesRefs | None = None | ||
) -> Block: | ||
""" | ||
Create a new block, with type inference propagate any values that are | ||
not specified | ||
|
@@ -219,7 +221,7 @@ def make_block(self, values, placement=None) -> Block: | |
|
||
# TODO: perf by not going through new_block | ||
# We assume maybe_coerce_values has already been called | ||
return new_block(values, placement=placement, ndim=self.ndim) | ||
return new_block(values, placement=placement, ndim=self.ndim, refs=refs) | ||
|
||
@final | ||
def make_block_same_class( | ||
|
@@ -372,7 +374,7 @@ def _split(self) -> list[Block]: | |
vals = self.values[slice(i, i + 1)] | ||
|
||
bp = BlockPlacement(ref_loc) | ||
nb = type(self)(vals, placement=bp, ndim=2) | ||
nb = type(self)(vals, placement=bp, ndim=2, refs=self.refs) | ||
new_blocks.append(nb) | ||
return new_blocks | ||
|
||
|
@@ -431,7 +433,9 @@ def _maybe_downcast(self, blocks: list[Block], downcast=None) -> list[Block]: | |
if downcast is None: | ||
return blocks | ||
|
||
return extend_blocks([b._downcast_2d(downcast) for b in blocks]) | ||
return extend_blocks( | ||
[b._downcast_2d(downcast) for b in blocks] # type: ignore[call-arg] | ||
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. Did something change here, or also a merge leftover? 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. Same yes, Updated everything now, should be good to go. |
||
) | ||
|
||
@final | ||
@maybe_split | ||
|
@@ -448,12 +452,15 @@ def convert( | |
self, | ||
*, | ||
copy: bool = True, | ||
using_cow: bool = False, | ||
) -> list[Block]: | ||
""" | ||
attempt to coerce any object types to better types return a copy | ||
of the block (if copy = True) by definition we are not an ObjectBlock | ||
here! | ||
""" | ||
if not copy and using_cow: | ||
return [self.copy(deep=False)] | ||
return [self.copy()] if copy else [self] | ||
|
||
# --------------------------------------------------------------------- | ||
|
@@ -2040,6 +2047,7 @@ def convert( | |
self, | ||
*, | ||
copy: bool = True, | ||
using_cow: bool = False, | ||
) -> list[Block]: | ||
""" | ||
attempt to cast any object types to better types return a copy of | ||
|
@@ -2048,6 +2056,8 @@ def convert( | |
if self.dtype != _dtype_obj: | ||
# GH#50067 this should be impossible in ObjectBlock, but until | ||
# that is fixed, we short-circuit here. | ||
if using_cow: | ||
return [self.copy(deep=False)] | ||
return [self] | ||
|
||
values = self.values | ||
|
@@ -2063,10 +2073,14 @@ def convert( | |
convert_period=True, | ||
convert_interval=True, | ||
) | ||
refs = None | ||
if copy and res_values is values: | ||
res_values = values.copy() | ||
elif res_values is values and using_cow: | ||
refs = self.refs | ||
|
||
res_values = ensure_block_shape(res_values, self.ndim) | ||
return [self.make_block(res_values)] | ||
return [self.make_block(res_values, refs=refs)] | ||
|
||
|
||
# ----------------------------------------------------------------- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -748,6 +748,41 @@ def test_head_tail(method, using_copy_on_write): | |
tm.assert_frame_equal(df, df_orig) | ||
|
||
|
||
def test_infer_objects(using_copy_on_write): | ||
df = DataFrame({"a": [1, 2], "b": "c", "c": 1, "d": "x"}) | ||
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. It might be worth adding a test where one of the object dtype columns actually gets converted? (eg change d 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. Good point, adjusted d in the two tests below to cover both cases. Good to go apart from that? |
||
df_orig = df.copy() | ||
df2 = df.infer_objects() | ||
|
||
if using_copy_on_write: | ||
assert np.shares_memory(get_array(df2, "a"), get_array(df, "a")) | ||
assert np.shares_memory(get_array(df2, "b"), get_array(df, "b")) | ||
|
||
else: | ||
assert not np.shares_memory(get_array(df2, "a"), get_array(df, "a")) | ||
assert not np.shares_memory(get_array(df2, "b"), get_array(df, "b")) | ||
|
||
df2.iloc[0, 0] = 0 | ||
df2.iloc[0, 1] = "d" | ||
if using_copy_on_write: | ||
assert not np.shares_memory(get_array(df2, "a"), get_array(df, "a")) | ||
assert not np.shares_memory(get_array(df2, "b"), get_array(df, "b")) | ||
tm.assert_frame_equal(df, df_orig) | ||
|
||
|
||
def test_infer_objects_no_reference(using_copy_on_write): | ||
df = DataFrame({"a": [1, 2], "b": "c", "c": 1, "d": "x"}) | ||
df = df.infer_objects() | ||
|
||
arr_a = get_array(df, "a") | ||
arr_b = get_array(df, "b") | ||
|
||
df.iloc[0, 0] = 0 | ||
df.iloc[0, 1] = "d" | ||
if using_copy_on_write: | ||
assert np.shares_memory(arr_a, get_array(df, "a")) | ||
assert not np.shares_memory(arr_b, get_array(df, "b")) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"kwargs", | ||
[ | ||
|
Uh oh!
There was an error while loading. Please reload this page.