BUG: re-render CSS with styler.apply and applymap non-cleared _todo
#39396
Conversation
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
what user facing effect does this have? can you add a whatsnew note
| (application method, *args, **kwargs) | ||
| """ | ||
| r = self |
There was a problem hiding this comment.
why do we not just call .clear() first?
There was a problem hiding this comment.
the _compute method deals only with the _todo list which is the composition of users apply and applymap methods. It populates the ctx object which is later used by _translate to render the HTML.
the clear method removes the _todo list as well as other objects such as Tooltips, the ctx object and the cell_context object from the Styler. You don't want to remove all these at this stage. I think you just want to empty the _todo list after its been 'done'.
There was a problem hiding this comment.
what user facing effect does this have? can you add a whatsnew note
Everytime a Styler is displayed in JupyterNotebook cell it calls render() and the css is processed (duplicated in the current version). Since HTML just reads the most recent CSS, duplicate CSS doesn't affect the display to user, but for debugging or network transferring the HTML it is best obviously to only generate the css once.
A common user pattern might be to build up styles in notebooks consecutively, meaning multiple cell renders will affect the html output.
Consider 3 cells, so ultimately 3 renders:
In [1]: df = pd.DataFrame({"A": [0, 1], "B": np.random.randn(2)})
s = df.style.set_uuid('A_').applymap(lambda x: 'color: green;')
s
Out[1]: <styled dataframe in green>
In [2]: s.set_uuid('B_').applymap(lambda x: 'font-weight: bold;')
Out [2]: <styled dataframe in bold green>
In [3]: print(s.render())
Out [3]: <style type="text/css" >
#T_B_row0_col0,#T_B_row0_col1,#T_B_row1_col0,#T_B_row1_col1{
color: green;
color: green;
font-weight: bold;
color: green;
font-weight: bold;
}</style>
This PR reduces the final output, without any other implications, to:
<style type="text/css" >
#T_B_row0_col0,#T_B_row0_col1,#T_B_row1_col0,#T_B_row1_col1{
color: green;
font-weight: bold;
}</style>
There was a problem hiding this comment.
ok can you add a test that replicates this
| assert len(s.ctx) > 0 | ||
| assert len(s._todo) == 0 | ||
| s._todo = [1] | ||
| s.clear() |
There was a problem hiding this comment.
add a line break / comment here
| assert len(s._todo) > 0 | ||
| assert s.tooltips | ||
| assert len(s.cell_context) > 0 | ||
| s = s._compute() |
There was a problem hiding this comment.
is there a reason we are calling private methods?
There was a problem hiding this comment.
this is legacy from my perspective. _compute is wrapped up within the render process. Calling just this, rather then say render is just an optimisation of the test, I believe.
There was a problem hiding this comment.
ok, yeah i fyou can just add some blank lines here so its a bit more obvious what is tested
| (application method, *args, **kwargs) | ||
| """ | ||
| r = self |
There was a problem hiding this comment.
ok can you add a test that replicates this
| assert len(s._todo) > 0 | ||
| assert s.tooltips | ||
| assert len(s.cell_context) > 0 | ||
| s = s._compute() |
There was a problem hiding this comment.
ok, yeah i fyou can just add some blank lines here so its a bit more obvious what is tested
|
@jreback, changes added, is green. |
|
thanks @attack68 |
2 participants
Styler.applyandapplymapduplicate CSS on re-render. #39395