BUG: do not hard-code matplotlib backend for boxplot
State: closed


Labels:
name: Visualization
description: plotting

Comments:
@datapythonista 
This was implemented in #27432, and was intentional. My preference was to delegate everything to the backends, but the final decision was to just delegate the plots called via accessor.

I'm happy with changing it for boxplot, even if I'd personally deprecate `DataFrame.boxplot()` in favor of `DataFrame.plot(kind='box')`.

@TomAugspurger thoughts on this change?
As long as delegating doesn’t get in the way of a future deprecation, then I’m fine with this. 
OK - what threw me off was that it's the only hard-coded backend in that file – the rest use whatever the user has set. Is there a reason for that discrepancy?
My understanding is that there are these ways of calling a boxplot from pandas:
```python
df.plot.box()
df.plot(kind='box')
df.boxplot()
pandas.plotting.boxplot(df)
```
The decision was to delegate to the backend anything that is not a `Series` or `DataFrame` method. In this case the 4 cases are implemented in `_core.py`, but the last one is the only that doesn't delegate.

I think we can merge this if it affects you. Otherwise I'd say better don't touch it, and focus on deprecating it. There was an initial discussion on deprecations in #26747, but the scope of the issue was too big and no decision was made on that. I guess deprecating `df.hist()` and `df.boxplot()` is controversial, but I guess people there will be agreement on `pandas.plotting.boxplot`.

I'll try to open an issue later today to make a decision on that.
Makes sense – so from the contents of this file it appears that plotting backends can override ``pandas.plotting.hist_series``, ``pandas.plotting.hist_frame``, ``pandas.plotting.boxplot_frame``, and ``pandas.plotting.boxplot_frame_groupby``, but not ``pandas.plotting.boxplot``. Was that your intent?
(I'm fine either way, I just think consistency here would make things less confusing for both users and backend authors)
Note also that the accessor is structured such that ``df.plot.hist()`` and ``df.plot.box()`` delegate to the backend via a separate code path, as reflected in the current implementation in https://github.com/altair-viz/altair_pandas/blob/master/altair_pandas/_core.py. Is that intentional?
> Makes sense – so from the contents of this file it appears that plotting backends can override `pandas.plotting.hist_series`, `pandas.plotting.hist_frame`, `pandas.plotting.boxplot_frame`, and `pandas.plotting.boxplot_frame_groupby`, but not `pandas.plotting.boxplot`. Was that your intent?

`pandas.plotting.hist_series`... are not expected to be called directly afaik, there are just the functions that are called when `Series.hist()`... are called.

There are many things in the current API that I think are explained by how things were implemented, and that are inconsistent. That's why I mentioned that I think what we should do is have a discussion on what to deprecate, and leave things simple, standardized and without inconsistencies.

Let's see what can be done, I think once there is agreement in what should be done, should be fast to implement the changes.
> there are just the functions that are called when Series.hist()... are called.

I don't think that's correct. From my read, when you call ``Series.hist()`` with the matplotlib backend, you get this code path: https://github.com/pandas-dev/pandas/blob/master/pandas/plotting/_matplotlib/hist.py#L17
I think you're right for `Series.plot.hist()`, but not for `Series.hist()`. See here: https://github.com/pandas-dev/pandas/blob/master/pandas/core/series.py#L4804

It's confusing, that's exactly why I think we should deprecate `Series.hist` (or just make an alias for `Series.plot.hist`, but I think alias also cause confusion).
OK – thanks for the clarification.

So would you recommend plotting backends define their own version of ``hist_series``, ``hist_dataframe``, ``boxplot_series``, and ``boxplot_dataframe``, or is the fact that they are able to be overridden (and ``boxplot`` is not) just an unintended consequence of an internal implementation?
If you want users to be able to plot with your backend when calling `Series.hist()`... you should implement them. I'd just call `plot(kind='hist')` there. I think there are subtle differences among the two, but we should take care of those in our end.

Making you implement those is not great, sorry. They won't be needed in the future, I think. But for now this is how the API works.
OK.

For what it's worth, if you're planning to deprecate them I think it would make more sense to have them produce the matplotlib plot in all cases (like ``boxplot`` and other top-level functions currently do).

The current behavior looks like this:
```
>>> pd.set_option('plotting.backend', 'altair_pandas')
>>> data = pd.DataFrame({'x': range(5)})
>>> pd.plotting.boxplot(data)
# produces matplotlib plot

>>> pd.plotting.boxplot_frame(data)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-48-0bc94aa14454> in <module>
      1 pd.set_option('plotting.backend', 'altair_pandas')
      2 data = pd.DataFrame({'x': range(5)})
----> 3 pd.plotting.boxplot_frame(data)

~/anaconda/lib/python3.6/site-packages/pandas/plotting/_core.py in boxplot_frame(self, column, by, ax, fontsize, rot, grid, figsize, layout, return_type, **kwds)
    407 ):
    408     plot_backend = _get_plot_backend()
--> 409     return plot_backend.boxplot_frame(
    410         self,
    411         column=column,

AttributeError: module 'altair_pandas' has no attribute 'boxplot_frame'
```
I've updated the PR to reflect how I think it should look given this new information.

Honestly, I'm fine either way as long as a consistent choice is made.
I'm going to close this, because the discussion is larger than the files I've touched here.