Fix undesired UX behavior of DataFrame output in IPython Notebook #10232

danieljl · 2015-05-29T17:09:03Z

shoyer · 2015-06-03T06:40:51Z

Could you please squash this to a single commit and add this to the release notes for 0.16.2? I think this would go in under "Other API changes": https://github.com/pydata/pandas/blob/master/doc/source/whatsnew/v0.16.2.txt

@jorisvandenbossche @jreback any thoughts on this issue? see also the associated issue #10231

danieljl · 2015-06-03T08:16:48Z

@shoyer I've squashed it into a single commit. Btw, I think this is more appropriate added to release notes under "Other enhancements", isn't it?

shoyer · 2015-06-03T08:19:49Z

doc/source/whatsnew/v0.16.2.txt

@@ -25,6 +25,8 @@ New features
 Other enhancements
 ^^^^^^^^^^^^^^^^^^

+- Fix undesired UX behavior of DataFrame output in IPython Notebook. (:issue:`10231`)


I might say: "Removed duplicate scroll bars in DataFrame HTML representation in IPython Notebook" (just to be a little more descriptive)

yeh, this needs a bit more explanation. Possibly even a screen shot. Its seems trivial, but people complain about UX changes the most!

shoyer · 2015-06-03T08:22:40Z

@danieljl This is a (small) API change, so I would put it under that category. "Other enhancements" is really for fully backwards compatible changes, e.g., adding a new optional keyword argument.

jorisvandenbossche · 2015-06-03T08:24:40Z

Personally I don't think it is needed to put it under API changes, as it is not really something that will break people's code, or something for which people have to adjust there code, it is only a visual change. I would keep 'API changes' for things people have to pay attention to if they use that in their code.

jorisvandenbossche · 2015-06-03T08:25:22Z

I was only thinking: how does this behave in older versions of IPython? (eg somebody who is still using IPython 2?)

shoyer · 2015-06-03T08:25:58Z

@jorisvandenbossche would you call this a bug fix? It's kind of a strange case -- we had a work around, which was resolved in a later version of IPython.

See the linked issue for screenshots.

jorisvandenbossche · 2015-06-03T08:31:22Z

Maybe we can detect the IPython version and dependable on that leave the max-width in or not?

@shoyer hmm, maybe also not a bug fix. I would keep it at an 'enhancement' of the visual output of a dataframe in the notebook. But does not really matter that much I think

jreback · 2015-06-03T12:43:04Z

I agree with @jorisvandenbossche I would detect ipython version and not make any change for ipython < 3.

Further, I think this needs a separate section in the what's new with a picture. UX changes needs to be announced loudly (even if its is a 'bug' fix).

jreback · 2015-06-05T21:40:47Z

@danieljl can you update with ipython version detection?

danieljl · 2015-06-06T04:36:32Z

@jreback Done. However, in IPython 2 horizontal scroll bar is still unscrollable unless the page is refreshed.

jreback · 2015-06-07T22:19:45Z

pandas/core/frame.py

@@ -22,6 +22,7 @@
 from numpy import nan as NA
 import numpy as np
 import numpy.ma as ma
+import IPython


this is not a required dependency, so do this in the actual function. surround with a try/except. if you cannot figure out the version (e.g. ipython is not installed), then use the new style is ok.

jreback · 2015-06-09T00:40:15Z

pandas/core/frame.py

+            try:
+                import IPython
+
+                if IPython.version_info < (3, 0, 0):


IPython.version_info < LooseVersion("3.0.0")

@jreback IPython.version_info is a tuple. Did you mean IPython.__version__? Btw, why is this more preferred?

jreback · 2015-06-09T00:53:40Z

Put this code as a method in HTMLFormatter.

This needs to be changed a bit. Instead of simply having the div_style wrap the self.html(...) method in __repr_html__, here, I would pass a new parameter to to_html, call it notebook (pass notebook=True, with a default value of notebook=False), pass this thru to the HTMLFormatter class (which sets it as an instance variable).

Then just call this code in write_result.

This will put all of the code in the correct place, and make it obvious what is going on.

danieljl · 2015-06-09T10:07:14Z

@jreback Should it be like this 3e0c851?

jreback · 2015-06-09T10:27:59Z

doc/source/whatsnew/v0.16.2.txt

@@ -25,6 +25,8 @@ New features
 Other enhancements
 ^^^^^^^^^^^^^^^^^^

+- Removed duplicate scroll bars in DataFrame HTML representation in IPython Notebook. (:issue:`10231`)


for notebook >= version 3.0.0

jreback · 2015-06-10T10:37:32Z

pls squash this to a single commit

jreback · 2015-06-10T10:40:44Z

doc/source/whatsnew/v0.16.2.txt

@@ -25,6 +25,8 @@ New features
 Other enhancements
 ^^^^^^^^^^^^^^^^^^

+- Removed duplicate scroll bars in DataFrame HTML representation for IPython 3.0.0 or later. (:issue:`10231`)


This needs some addtl explanation w.r.t. max_rows. IOW, I find that using pd.set_setoption('max_rows,None) is now a nice way to use the notebook (for a terminal I have this set to a smaller value, however). maybe mention of same in options.rst

jreback · 2015-06-10T10:42:17Z

@shoyer @jorisvandenbossche @TomAugspurger @sinhrks

thoughts?

TomAugspurger · 2015-06-10T12:11:24Z

looks good.

sinhrks · 2015-06-10T12:24:33Z

lgtm.

shoyer · 2015-06-10T13:27:43Z

Yep, looks pretty good to me.

jorisvandenbossche · 2015-06-10T13:35:22Z

Yes, here as well. I quickly tested it, and although I was used to having shorter outputs of dataframes in my notebook workflow (now it show about 30 rows, with this change the full 60 rows of a truncated dataframe repr will be visible, leading to a longer output), I think I will get used to the change.

The only disadvantage is that I find the "toggle output scrolling" feature of IPython (with this, you see around 9 rows) rather ugly (the way too heavy shadow).
And that I did find the 30 rows you see now a better height as the 60 rows (but that has to do with our default options for max_rows, so another discussion)

danieljl · 2015-06-10T14:31:31Z

@jreback Squashed

shoyer · 2015-06-10T14:35:17Z

@jorisvandenbossche agreed, it is a little ugly to have a drop shadow here. But this is still better than double scroll bars.

jreback · 2015-06-10T14:36:50Z

@danieljl that looks good. can you expand the whatsnew note a bit? approach this from the perspective of someone reading this, and since this is a UX change, we want to be obvious/clear what exactly this is.

danieljl · 2015-06-10T15:26:57Z

@jreback Whatsnew updated :)

jorisvandenbossche · 2015-06-10T15:28:35Z

One comment: this adds a new notebook kwarg to DataFrame.to_html, which I don't think is really useful to be exposed to the user?
Isn't it possible to not add it? (eg leave the logic of that in __repr__ instead of in HTMLFormatter)

danieljl · 2015-06-10T15:37:48Z

@jorisvandenbossche Yup, initially I implemented it in _repr_html_. But I refactored it as suggested by @jreback in #10232 (comment).

jreback · 2015-06-10T15:56:26Z

I guess we could make it _notebook e.g. a private argument. This totally belongs in the HTMLFormatter class (as for example you could then sub-class it and/or use it somewhere else that you are displaying to a notebook)

jorisvandenbossche · 2015-06-10T16:11:13Z

OK, agreed. Other option is to call HTMLFormatter (or DataFrameFormatter) directly from __repr__ instead of to_html, like

return fmt.DataFrameFormatter(self, max_rows=max_rows, max_cols=max_cols,
                              show_dimensions=show_dimensions,
                              notebook=True).to_html()

or if you think this is useful in to_html as well (and not only in HTMLFormatter), let's just document it in the docstring (and maybe use a more generic name like div?)

jreback · 2015-06-10T16:20:42Z

how about then we make this an argument to to_html instead then. (I still would use notebook as it conveys, hey, we want to output this to a particular medium, html, but needs a special wrapper because its in the notebook)

danieljl · 2015-06-11T06:11:02Z

So should the code be refactored again?

jorisvandenbossche · 2015-06-11T12:55:03Z

@danieljl I think the latest what @jreback said was to keep it in to_html, but then it should get added to the docstring, that is all

jreback · 2015-06-11T13:08:02Z

pandas/core/frame.py

@@ -1387,7 +1387,8 @@ def to_html(self, buf=None, columns=None, col_space=None, colSpace=None,
                                           escape=escape,
                                           max_rows=max_rows,
                                           max_cols=max_cols,
-                                           show_dimensions=show_dimensions)
+                                           show_dimensions=show_dimensions,
+                                           notebook=notebook)


here pass directly to .to_html() (and add in the doc-string). That way this is not relevant to DataFrameFormatter, just to the HTMLFormatter (which is created in the .to_html call)

danieljl · 2015-06-11T15:19:04Z

If everything's okay, I will squash it again.

jreback · 2015-06-11T15:23:36Z

lgtm. go ahead and squash

…das-dev#10231

danieljl · 2015-06-11T15:32:07Z

Done

jreback · 2015-06-11T15:39:20Z

merged via 814dbe8

thanks!

shoyer added this to the 0.16.2 milestone Jun 3, 2015

shoyer added the IO HTML read_html, to_html, Styler.apply, Styler.applymap label Jun 3, 2015

danieljl force-pushed the fix-10231 branch 3 times, most recently from 8941b83 to 1aa2c04 Compare June 3, 2015 08:08

shoyer reviewed Jun 3, 2015
View reviewed changes

jreback reviewed Jun 7, 2015
View reviewed changes

jreback added the Output-Formatting __repr__ of pandas objects, to_string label Jun 7, 2015

jreback reviewed Jun 9, 2015
View reviewed changes

danieljl force-pushed the fix-10231 branch 2 times, most recently from 7e73bec to 3e0c851 Compare June 9, 2015 10:06

jreback reviewed Jun 9, 2015
View reviewed changes

jorisvandenbossche mentioned this pull request Jun 9, 2015

REL: 0.16.2 #10307

Closed

jreback reviewed Jun 10, 2015
View reviewed changes

shoyer mentioned this pull request Jun 10, 2015

Custom column formatters for HTML in IPython, e.g. can show np 2D-array as image #9579

Closed

danieljl force-pushed the fix-10231 branch from e6f8095 to e84f631 Compare June 10, 2015 14:28

danieljl force-pushed the fix-10231 branch from e84f631 to 8c60691 Compare June 10, 2015 15:24

jreback reviewed Jun 11, 2015
View reviewed changes

Fix undesired UX behavior of DataFrame output in IPython Notebook pan…

26421eb

…das-dev#10231

danieljl force-pushed the fix-10231 branch from 29604d5 to 26421eb Compare June 11, 2015 15:31

jreback closed this Jun 11, 2015

jreback mentioned this pull request Aug 3, 2015

Undesired UX behavior of DataFrame output in IPython Notebook #10231

Closed

jorisvandenbossche mentioned this pull request Aug 3, 2015

Vertical and horizontal scrollbar not both visible in toggled output jupyter/notebook#254

Closed

jorisvandenbossche mentioned this pull request Jul 4, 2016

_repr_html_ creates extra scrollbar in ipython notebook #9268

Closed

Fix undesired UX behavior of DataFrame output in IPython Notebook #10232

Fix undesired UX behavior of DataFrame output in IPython Notebook #10232

Conversation

danieljl commented May 29, 2015

shoyer commented Jun 3, 2015

danieljl commented Jun 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer commented Jun 3, 2015

jorisvandenbossche commented Jun 3, 2015

jorisvandenbossche commented Jun 3, 2015

shoyer commented Jun 3, 2015

jorisvandenbossche commented Jun 3, 2015

jreback commented Jun 3, 2015

jreback commented Jun 5, 2015

danieljl commented Jun 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jun 9, 2015

danieljl commented Jun 9, 2015

Choose a reason for hiding this comment

jreback commented Jun 10, 2015

Choose a reason for hiding this comment

jreback commented Jun 10, 2015

TomAugspurger commented Jun 10, 2015

sinhrks commented Jun 10, 2015

shoyer commented Jun 10, 2015

jorisvandenbossche commented Jun 10, 2015

danieljl commented Jun 10, 2015

shoyer commented Jun 10, 2015

jreback commented Jun 10, 2015

danieljl commented Jun 10, 2015

jorisvandenbossche commented Jun 10, 2015

danieljl commented Jun 10, 2015

jreback commented Jun 10, 2015

jorisvandenbossche commented Jun 10, 2015

jreback commented Jun 10, 2015

danieljl commented Jun 11, 2015

jorisvandenbossche commented Jun 11, 2015

Choose a reason for hiding this comment

danieljl commented Jun 11, 2015

jreback commented Jun 11, 2015

danieljl commented Jun 11, 2015

jreback commented Jun 11, 2015