Fix undesired UX behavior of DataFrame output in IPython Notebook
State: closed

Closes #10231


Labels:
name: Output-Formatting
description: __repr__ of pandas objects, to_string
name: IO HTML
description: read_html, to_html, Styler.apply, Styler.applymap

Comments:
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

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

@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.

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.

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

@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.

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

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).

@danieljl can you update with ipython version detection?

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

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](https://github.com/pydata/pandas/blob/master/pandas/core/frame.py#L541), 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.

@jreback Should it be like this 3e0c851?

pls squash this to a single commit

@shoyer @jorisvandenbossche @TomAugspurger @sinhrks 

thoughts?

looks good.

lgtm.

Yep, looks pretty good to me. 

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)

@jreback Squashed

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

@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.

@jreback Whatsnew updated :)

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`)

@jorisvandenbossche Yup, initially I implemented it in `_repr_html_`. But I refactored it as suggested by @jreback in https://github.com/pydata/pandas/pull/10232#issuecomment-110184910.

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)

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`?)

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)

So should the code be refactored again?

@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

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