Skip to content

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

Merged
merged 10 commits into from
Mar 5, 2021
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ Other
- Bug in :class:`Styler` where rendered HTML was missing a column class identifier for certain header cells (:issue:`39716`)
- Bug in :meth:`Styler.background_gradient` where text-color was not determined correctly (:issue:`39888`)
- Bug in :meth:`DataFrame.equals`, :meth:`Series.equals`, :meth:`Index.equals` with object-dtype containing ``np.datetime64("NaT")`` or ``np.timedelta64("NaT")`` (:issue:`39650`)
- Bug in :class:`Styler` copy methods which were not updated for recently introduced attributes (:issue:`39708`)
Copy link
Contributor

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 listed

Copy link
Contributor

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)

Copy link
Contributor

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



.. ---------------------------------------------------------------------------
Expand Down
16 changes: 15 additions & 1 deletion pandas/io/formats/style.py
Original file line number Diff line number Diff line change
Expand Up @@ -783,15 +783,29 @@ def _copy(self, deepcopy: bool = False) -> Styler:
precision=self.precision,
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I have never had use to copy a Styler. But I can see a use case where, in a notebook or report, you might have a dataframe that has some preliminary, shared styling and then some divergence where you highlight different properties as part of a narrative.

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,
Copy link
Contributor

@attack68 attack68 Feb 22, 2021

Choose a reason for hiding this comment

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

the uuid is suffixed with "_" so need to trim this off here self.uuid[:-1]
edit: actually it might be cleaner and always work if the uuid is manually overwritten after class instantiation.

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,
Copy link
Contributor

Choose a reason for hiding this comment

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

ignore the uuid_len since the uuid is explicitly provided

)

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

Choose a reason for hiding this comment

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

think hidden_columns is a list, does it need deepcopy?

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

return styler

def __copy__(self) -> Styler:
Expand Down
111 changes: 107 additions & 4 deletions pandas/tests/io/formats/test_style.py
Original file line number Diff line number Diff line change
Expand Up @@ -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%}")
Copy link
Contributor

Choose a reason for hiding this comment

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

not making any changes to _todo[]. maybe add .apply(..)


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

Choose a reason for hiding this comment

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

uuid is the key variable, uuid_len is only used in construction of uuid test for that instead.

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

Choose a reason for hiding this comment

The 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
Expand Down