PLT: Cleaner plotting backend API, and unify Series and DataFrame accessors
State: closed

- [X] xref #26747
- [ ] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

The main changes are:

The API that backends need to implement is simply a function `plot(data, kind, **kwargs)`. After testing different approaches (abstract class previously mentioned and others), I think this is what makes things simpler in both ends pandas and the backends. For `hvplot` `hvPlot.__call__` is already what needs to be implemented, and this change will mainly require to delete a bit of code. For `pdvega` a bit more work will be needed, but this will help remove a significant amount of repetitive code.

Second thing, more in the pandas side, I unify the accessors of `Series` and `DataFrame`. There will be minor changes, like unified documentation (which I think it's great, but docstrings will have to be generic), and also `Series.plot.scatter` will exist (which now doesn't) but will generate an exception if called. Not a big deal I'd say, because now it's already possible to do `Series.plot(kind='scatter')`, so it'll just make both cases consistent. Same things apply to `hexbin`.

CC: @pandas-dev/pandas-core @philippjfr @jakevdp

Labels:
name: Visualization
description: plotting
name: Blocker
description: Blocking issue or pull request for an upcoming release
name: Clean

Comments:
I'm still a bit unclear on how the dispatching to backends will work. IIUC, on master we structure things as `DataFrame.plot.__call__(kind, ...)`, which selects the right method based on the `kind`. So `DataFrame.plot(kind='line')` calls `DataFrame.plot.line` which does its own thing.

Now we're going to change things a bit so that `DataFrame.plot.line` calls `DataFrame.plot.__call__` with `kind=line`.

Does that have a direct impact on how backends will provide their own implementations?
> Now we're going to change things a bit so that `DataFrame.plot.line` calls `DataFrame.plot.__call__` with `kind=line`.

Not exactly. I didn't modify that part (besides merging the accessors for Series and DataFrame). In master `DataFrame.plot.line` calls the accessor `line` method, and the `line` method calls `__call__`. And this didn't change.

The `__call__` method was calling `plot_frame` and `plot_series`, and those were calling `plot`. `plot was instantiating/calling `LinePlot()` (`LinePlot` was the public part of the backend). This is the part that changed.

Now `__call__` will call `plot` in the backend (`plot` is the public part of the backend). And `plot` will instantiate and call `LinePlot` and the other classes. I think this is much better, since all backends will run some common code first no matter the plot kind.

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=h1) Report
> Merging [#27009](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/f0919f272d9614058b5ebb5e0664d1ac6f23540f?src=pr&el=desc) will **decrease** coverage by `50.16%`.
> The diff coverage is `41.79%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/27009/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           master   #27009       +/-   ##
===========================================
- Coverage   91.99%   41.83%   -50.17%     
===========================================
  Files         180      180               
  Lines       50774    50673      -101     
===========================================
- Hits        46712    21199    -25513     
- Misses       4062    29474    +25412
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `?` | |
| #single | `41.83% <41.79%> (-0.11%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `34.99% <100%> (-62.02%)` | :arrow_down: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `45.45% <100%> (-48.22%)` | :arrow_down: |
| [pandas/plotting/\_matplotlib/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL19faW5pdF9fLnB5) | `56% <26.66%> (-44%)` | :arrow_down: |
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `46.87% <44%> (-42.52%)` | :arrow_down: |
| [pandas/io/formats/latex.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvbGF0ZXgucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/gcs.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2djcy5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/sas/sas\_constants.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXNfY29uc3RhbnRzLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/core/groupby/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9jYXRlZ29yaWNhbC5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/plotting.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvcGxvdHRpbmcucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| ... and [136 more](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=footer). Last update [f0919f2...f00ec30](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@ad2e98c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `36.84%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/27009/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master   #27009   +/-   ##
=========================================
  Coverage          ?   41.82%           
=========================================
  Files             ?      180           
  Lines             ?    50683           
  Branches          ?        0           
=========================================
  Hits              ?    21197           
  Misses            ?    29486           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #single | `41.82% <36.84%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `26.66% <ø> (ø)` | |
| [pandas/plotting/\_matplotlib/timeseries.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL3RpbWVzZXJpZXMucHk=) | `12.75% <0%> (ø)` | |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `45.45% <100%> (ø)` | |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `34.99% <100%> (ø)` | |
| [pandas/plotting/\_matplotlib/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/27009/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL19faW5pdF9fLnB5) | `60% <33.33%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=footer). Last update [ad2e98c...2597bc9](https://codecov.io/gh/pandas-dev/pandas/pull/27009?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Still working on the failing tests here. Of the two that are failing locally, the problem can be reproduced with this simple example:
```python
import pandas

df = pandas.DataFrame({'a': [.19, .78, .27],
                       'b': [.62, .77, .80]})
df.plot()
df.plot(y='a')
```

In master this generates two plots, but in this branch is generating a single plot for both calls. Couldn't figure out why yet.
> In master this generates two plots, but in this branch is generating a single plot for both calls.

I think it's because on master, ``plot_frame`` lacks the check for an existing figure:(i.e. ``if ax is None and len(plt.get_fignums()) > 0: ...``). ``plot_series`` does have this check, so that multiple series plot statements end up on a single figure. I'm not certain whether this difference in behavior between dataframe plotting and series plotting is intentional.

After this change, both dataframes and series use this code.
> I'm not certain whether this difference in behavior between dataframe plotting and series plotting is intentional.

@jakevdp It's in any case a long standing behaviour (with quite some disagreement which of the two is better): https://github.com/pandas-dev/pandas/issues/8776
@datapythonista do you have time the coming days to try to fix the tests?
Thanks for the reviews, and thanks a lot for the comment @jakevdp, didn't realize of that behavior, but that made me understand the problem, I was very confused.

The problem was that I moved the code making the plot be in the same figure for Series to the backend, and the backend may receive a Series for DataFrame plots since we convert the data in some cases. I added an extra parameter `new_plot`  for the backend. It'll surely make sense to continue the discussion on #8776, but not making any functional change in this refactoring.

@jorisvandenbossche I kept the original order of the methods in this PR as you suggested, but looks like git is not smart enough in this case and the diff is still huge.
> @jorisvandenbossche I kept the original order of the methods in this PR as you suggested, but looks like git is not smart enough in this case and the diff is still huge.

Sorry for letting you do this then, I hoped git was smarter :) (but I think it did *something*)

No worries, I think the change made sense, but I guess there are too many lines deleted at the beginning of the file for git to match correctly.

There are couple of tests failing with matplotlib 2.2, but other than that things look good now.
All green here finally. :)

I think all comments from reviews have been addressed too, let me know if there is anything else.
Hello @datapythonista! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



















There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2019-07-03 14:07:47 UTC
@datapythonista how are the changes in this PR affecting the misc plots? (since the failing tests seem to be related to those)
No idea, I think the test started to fail after some changes to the documentation and comments, or a merge, doesn't make any sense. The changes here shouldn't affect andrews_curves at all. Can't reproduce locally, so I need to debug in the CI.
Failing example:
```python
from pandas import DataFrame
from pandas.plotting import parallel_coordinates

df = DataFrame({"feat": [i for i in range(30)],
                "class": [2 for _ in range(10)] +
                         [3 for _ in range(10)] +
                         [1 for _ in range(10)]})
ax = parallel_coordinates(df, 'class', sort_labels=True)
polylines, labels = ax.get_legend_handles_labels()
color_label_tuples = \
    zip([polyline.get_color() for polyline in polylines], labels)
ordered_color_label_tuples = sorted(color_label_tuples,
                                    key=lambda x: x[1])
ordered_color_label_tuples
```

Locally returns:
```
[([0.417022004702574, 0.7203244934421581, 0.00011437481734488664], '1'),
 ([0.43599490214200376, 0.025926231827891333, 0.5496624778787091], '2'),
 ([0.5488135039273248, 0.7151893663724195, 0.6027633760716439], '3')]
```
Which is correct, but in the Mac build in the CI seems to return:
```
[([0.417022004702574, 0.7203244934421581, 0.00011437481734488664], '1'),
 ([0.43599490214200376, 0.025926231827891333, 0.5496624778787091], '2'),
 ([0.5488135039273248, 0.7151893663724195, 0.6027633760716439], '3'),
 ('#1f77b4', 'None')]
```
No idea where this mysterious last element comes from. If anyone with a Mac can reproduce it locally that can makes things easy.

The CI was green, and this started to fail after updating comments and the docs, and adding a warning for `Series.plot` with positional arguments. I'm temporary removing the warning to see if it is causing the error.


>  I'm temporary removing the warning to see if it is causing the error.

It didn't help .. (the mac build still has the additional parallel_coordinates error).

I am fine with skipping that test on Mac to get this merged, and try to fix in a follow-up.


Very weird, the other commit when this started to fail was just on comments and docs. Will continue tomorrow.
Looks like this is the commit that is making the parallel_coordinates test fail: https://github.com/pandas-dev/pandas/pull/27204/commits/16d1f88cfcb7b30f86637422f550db4c50d58185

I'll remove it for now if that's all right.
> Looks like this is the commit that is making the parallel_coordinates test fail: 16d1f88

It might be that putting it *in* the class fixes it. The setup method of that class imports matplotlib (which might influence if that is imported before pyplot).
The warning is tested in another test anyway, it's redundant, so I think it's not a problem to remove that test.

@jreback you requested changes, can you have a look?
thanks @datapythonista very nice!

prob want to have a look at built docs / doc-strings to make sure everything renderning correctly.
Nice work @datapythonista! Anything else you want to get to for the release candidate? I'm hoping to tag it in ~7 hours.
I just commented on the issue that I think we should decide about the scope of the plotting backend (include misc plotting functions or not) for 0.25.0, but that doesn't need to be done for the RC. 
will check the docs once they're built.

Thanks Tom, we started to work on hvplot, and I fixed here couple of minor things that were creating problems for the backends. I think everything should be all right on this now, but if there is anything should be very small and can wait after the RC.

The main thing pending on this affecting the release is what Joris just mentioned on what we delegate to the backends. We can discuss that in #26747.