-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Incomplete Styler copy methods fix (#39708) #39975
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 1 commit
af84c69
6e4927f
d916e66
beae319
88ebd70
be9676a
68a19ec
dc338d5
b3dc84d
c300622
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 |
---|---|---|
|
@@ -783,15 +783,29 @@ def _copy(self, deepcopy: bool = False) -> Styler: | |
precision=self.precision, | ||
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. is there a reason we actually care about deepcopy, IOW why are we not just always a shallow copy? is this actually used somewhere? 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. Personally, I have never had use to copy a I think, off the top of my head, in this scenario you would need a deepcopy to prevent any updates from intermingling with each other, since much of the core styling I think comes from the attributes where the shallow copy will have only single shared pointed reference. |
||
caption=self.caption, | ||
uuid=self.uuid, | ||
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. the |
||
table_styles=self.table_styles, | ||
table_attributes=self.table_attributes, | ||
cell_ids=self.cell_ids, | ||
na_rep=self.na_rep, | ||
uuid_len=self.uuid_len, | ||
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. ignore the |
||
) | ||
|
||
styler.hidden_index = self.hidden_index | ||
|
||
if deepcopy: | ||
styler.ctx = copy.deepcopy(self.ctx) | ||
styler._todo = copy.deepcopy(self._todo) | ||
styler.table_styles = copy.deepcopy(self.table_styles) | ||
styler.hidden_columns = copy.deepcopy(self.hidden_columns) | ||
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. think |
||
styler.cell_context = copy.deepcopy(self.cell_context) | ||
styler.tooltips = copy.deepcopy(self.tooltips) | ||
else: | ||
styler.ctx = self.ctx | ||
styler._todo = self._todo | ||
styler.table_styles = self.table_styles | ||
styler.hidden_columns = self.hidden_columns | ||
styler.cell_context = self.cell_context | ||
styler.tooltips = self.tooltips | ||
jreback marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return styler | ||
|
||
def __copy__(self) -> Styler: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,28 +87,131 @@ def test_update_ctx_flatten_multi_and_trailing_semi(self): | |
} | ||
assert self.styler.ctx == expected | ||
|
||
def test_copy(self): | ||
@pytest.mark.parametrize("do_changes", [True, False]) | ||
@pytest.mark.parametrize("do_render", [True, False]) | ||
def test_copy(self, do_changes, do_render): | ||
# Updated in GH39708 | ||
# Change some defaults (to check later if they are copied) | ||
if do_changes: | ||
style = [{"selector": "th", "props": [("foo", "bar")]}] | ||
self.styler.set_table_styles(style) | ||
attributes = 'class="foo" data-bar' | ||
self.styler.set_table_attributes(attributes) | ||
self.styler.uuid_len += 1 | ||
self.styler.hidden_index = not self.styler.hidden_index | ||
self.styler.hide_columns("A") | ||
classes = pd.DataFrame( | ||
[["favorite-val red", ""], [None, "blue my-val"]], | ||
index=self.df.index, | ||
columns=self.df.columns, | ||
) | ||
self.styler.set_td_classes(classes) | ||
ttips = pd.DataFrame( | ||
data=[["Favorite", ""], [np.nan, "my"]], | ||
columns=self.df.columns, | ||
index=self.df.index, | ||
) | ||
self.styler.set_tooltips(ttips) | ||
self.styler.cell_ids = not self.styler.cell_ids | ||
self.styler.format("{:.2%}") | ||
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. not making any changes to _todo[]. maybe add |
||
|
||
if do_render: | ||
self.styler.render() | ||
|
||
s2 = copy.copy(self.styler) | ||
|
||
# Check for identity | ||
assert self.styler is not s2 | ||
assert self.styler.ctx is s2.ctx # shallow | ||
assert self.styler._todo is s2._todo | ||
|
||
assert self.styler.table_styles is s2.table_styles | ||
assert self.styler.hidden_columns is s2.hidden_columns | ||
assert self.styler.cell_context is s2.cell_context | ||
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. can you group the shallow variables in test to make it clearer? |
||
assert self.styler.tooltips is s2.tooltips | ||
if do_changes: # self.styler.tooltips is not None | ||
assert self.styler.tooltips.tt_data is s2.tooltips.tt_data | ||
|
||
# Check for equality (and changes in referenced objects) | ||
self.styler._update_ctx(self.attrs) | ||
self.styler.highlight_max() | ||
assert self.styler.ctx == s2.ctx | ||
assert self.styler._todo == s2._todo | ||
assert self.styler.table_styles == s2.table_styles | ||
assert self.styler.table_attributes == s2.table_attributes | ||
assert self.styler.cell_ids == s2.cell_ids | ||
assert self.styler.uuid_len == s2.uuid_len | ||
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.
|
||
assert self.styler.hidden_index == s2.hidden_index | ||
assert self.styler.hidden_columns == s2.hidden_columns | ||
assert self.styler.cell_context == s2.cell_context | ||
if do_changes: # self.styler.table_style is not None | ||
self.styler.table_styles[0]["selector"] = "ti" | ||
assert self.styler.table_styles == s2.table_styles | ||
if do_changes: # self.styler.tooltips is not None | ||
tm.assert_frame_equal(self.styler.tooltips.tt_data, s2.tooltips.tt_data) | ||
|
||
@pytest.mark.parametrize("do_changes", [True, False]) | ||
@pytest.mark.parametrize("do_render", [True, False]) | ||
def test_deepcopy(self, do_changes, do_render): | ||
# Updated in GH39708 | ||
# Change some defaults (to check later if they are copied) | ||
if do_changes: | ||
style = [{"selector": "th", "props": [("foo", "bar")]}] | ||
self.styler.set_table_styles(style) | ||
attributes = 'class="foo" data-bar' | ||
self.styler.set_table_attributes(attributes) | ||
self.styler.uuid_len += 1 | ||
self.styler.hidden_index = not self.styler.hidden_index | ||
self.styler.hide_columns("A") | ||
classes = pd.DataFrame( | ||
[["favorite-val red", ""], [None, "blue my-val"]], | ||
index=self.df.index, | ||
columns=self.df.columns, | ||
) | ||
self.styler.set_td_classes(classes) | ||
ttips = pd.DataFrame( | ||
data=[["Favorite", ""], [np.nan, "my"]], | ||
columns=self.df.columns, | ||
index=self.df.index, | ||
) | ||
self.styler.set_tooltips(ttips) | ||
self.styler.cell_ids = not self.styler.cell_ids | ||
self.styler.format("{:.2%}") | ||
|
||
if do_render: | ||
self.styler.render() | ||
|
||
def test_deepcopy(self): | ||
s2 = copy.deepcopy(self.styler) | ||
|
||
# Check for non-identity | ||
assert self.styler is not s2 | ||
assert self.styler.ctx is not s2.ctx | ||
assert self.styler._todo is not s2._todo | ||
|
||
assert self.styler.hidden_columns is not s2.hidden_columns | ||
assert self.styler.cell_context is not s2.cell_context | ||
if do_changes: # self.styler.table_style is not None | ||
assert self.styler.table_styles is not s2.table_styles | ||
if do_changes: # self.styler.tooltips is not None | ||
assert self.styler.tooltips is not s2.tooltips | ||
assert self.styler.tooltips.tt_data is not s2.tooltips.tt_data | ||
|
||
# Check for equality (and changes in original objects) | ||
self.styler._update_ctx(self.attrs) | ||
self.styler.highlight_max() | ||
assert self.styler.ctx != s2.ctx | ||
assert s2._todo == [] | ||
assert self.styler._todo != s2._todo | ||
assert self.styler.table_styles == s2.table_styles | ||
assert self.styler.table_attributes == s2.table_attributes | ||
assert self.styler.cell_ids == s2.cell_ids | ||
assert self.styler.uuid_len == s2.uuid_len | ||
assert self.styler.hidden_index == s2.hidden_index | ||
assert self.styler.hidden_columns == s2.hidden_columns | ||
assert self.styler.cell_context == s2.cell_context | ||
if do_changes: # self.styler.table_style is not None | ||
self.styler.table_styles[0]["selector"] = "ti" | ||
assert self.styler.table_styles != s2.table_styles | ||
if do_changes: # self.styler.tooltips is not None | ||
tm.assert_frame_equal(self.styler.tooltips.tt_data, s2.tooltips.tt_data) | ||
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. possible to wrap the two test cases copy and deepcopy so that dont repeat so much code? |
||
|
||
def test_clear(self): | ||
# updated in GH 39396 | ||
|
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 move this up to where other
Styler
Items are listedThere 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.
this may not be necessary if this changes were in 1.3 (which they might be); you can add this issue number with another change for Styler (e.g. where the attributes were changed)
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.
this has to be 1.3, since tooltips is the most recent and thats in 1.3.0, referenced in this PR