PLOT: Split matplotlib specific code from pandas plotting
State: closed

- [X] xref #14130
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

In order to be able to change pandas plotting backend, I think we need 3 main things:
1. Decouple the current matplotlib backend from the pandas plotting system, and just call it using a defined public API
2. Implement an option to select the backend (e.g. `pandas.set_option('plotting.backend', 'pandas.matplotlib')`)
3. Have other alternative backends which define the same API as the current matplotlib one

This PR is splitting most of the matplotlib backend code into a new `_matplotlib.py` file. Some matplotlib code still lives in `plotting/_core.py`, but would be nice to have early feedback on whether everybody is happy with this approach before moving forward. CC: @jreback @TomAugspurger 

Labels:
name: Refactor
description: Internal refactoring of code
name: Visualization
description: plotting

Comments:
Thanks, will take a look later. In case it's useful now, I started https://github.com/TomAugspurger/pandas/commit/b07aba28a37b0291fd96a1f571848a7be2b6de8d, which adds a config option for plotting, but never got to submitting a PR.
Totally on-board with the concept.  What other backends do you expect (either implemented here or downstream)?  My understanding is many of the other graphics libraries wrap matplotlib themselves.
I know of two libraries that have reimplemented the pd.DataFrame.plot API

- https://github.com/altair-viz/pdvega/ (altair)
- https://hvplot.pyviz.org (bokeh)
I think Phillip started hvplot with the discussions of this issue, but as we didn't finish this work, it ended monkeypatching pandas.

The goal here is that projects like the ones Tom mentioned, and others like https://github.com/PatrikHlobil/Pandas-Bokeh can be implemented simpler and in a standard way, so we make the life of devs and users easier.

Personally, I'd at some point split the matplotlib backend from pandas and have it as a third party package. But that's for the long term, and unrelated to this PR.
For what I could see in the code, the API that pandas plotting backends need to provide are:

- `LinePlot`
- `BarPlot`
- `BarhPlot`
- `HistPlot`
- `BoxPlot`
- `KdePlot`
- `AreaPlot`
- `PiePlot` (only for `pandas.Series`)
- `ScatterPlot` (only for `pandas.DataFrame`)
- `HexBinPlot` (only for `pandas.DataFrame`)
- `hist_series`
- `hist_frame`
- `boxplot_frame`
- `boxplot_frame_groupby`

The implementation in this PR moves all the `matplotlib` code into `pandas.plotting.matplotlib`. And keeps all the public documentation in pandas (outside of the matplotlib backend). In a follow up PR should be simple to add an option to change the backend to another respecting the same API as the matplotlib one.

Any comment appreciated.

CC: @tacaswell, @jakevdp, @philippjfr, @PatrikHlobil
This is great - thanks for working on it!
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26414?src=pr&el=h1) Report
> Merging [#26414](https://codecov.io/gh/pandas-dev/pandas/pull/26414?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/ff4437eb3b27440ea2304a8613369ebec0b8b72a?src=pr&el=desc) will **decrease** coverage by `49.94%`.
> The diff coverage is `17%`.

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

```diff
@@             Coverage Diff             @@
##           master   #26414       +/-   ##
===========================================
- Coverage   91.69%   41.74%   -49.95%     
===========================================
  Files         174      178        +4     
  Lines       50739    50739               
===========================================
- Hits        46524    21181    -25343     
- Misses       4215    29558    +25343
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `?` | |
| #single | `41.74% <17%> (+0.4%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26414?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/plotting/matplotlib/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL21hdHBsb3RsaWIvX19pbml0X18ucHk=) | `100% <100%> (ø)` | |
| [pandas/plotting/matplotlib/boxplot.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL21hdHBsb3RsaWIvYm94cGxvdC5weQ==) | `15.02% <15.02%> (ø)` | |
| [pandas/plotting/matplotlib/main.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL21hdHBsb3RsaWIvbWFpbi5weQ==) | `15.19% <15.19%> (ø)` | |
| [pandas/plotting/matplotlib/hist.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL21hdHBsb3RsaWIvaGlzdC5weQ==) | `21.38% <21.38%> (ø)` | |
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `44.3% <27.9%> (-39.6%)` | :arrow_down: |
| [pandas/io/formats/latex.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvbGF0ZXgucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/sas/sas\_constants.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/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/26414/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9jYXRlZ29yaWNhbC5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/plotting.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvcGxvdHRpbmcucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| ... and [140 more](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26414?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/26414?src=pr&el=footer). Last update [ff4437e...0e45c2a](https://codecov.io/gh/pandas-dev/pandas/pull/26414?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/26414?src=pr&el=h1) Report
> Merging [#26414](https://codecov.io/gh/pandas-dev/pandas/pull/26414?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/5b10e31fbf2707f354c2ffe287b434f10a8651d1?src=pr&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `75.96%`.

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

```diff
@@            Coverage Diff            @@
##           master   #26414     +/-   ##
=========================================
- Coverage   91.88%   91.79%   -0.1%     
=========================================
  Files         174      179      +5     
  Lines       50699    50763     +64     
=========================================
+ Hits        46587    46600     +13     
- Misses       4112     4163     +51
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.33% <75.96%> (-0.09%)` | :arrow_down: |
| #single | `41.31% <3.43%> (-0.7%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26414?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/plotting/\_matplotlib/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL2NvbnZlcnRlci5weQ==) | `58.43% <ø> (ø)` | |
| [pandas/plotting/\_matplotlib/compat.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL2NvbXBhdC5weQ==) | `84.61% <ø> (ø)` | |
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/plotting/\_matplotlib/style.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL3N0eWxlLnB5) | `85.96% <100%> (ø)` | |
| [pandas/tseries/plotting.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvcGxvdHRpbmcucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [pandas/plotting/\_matplotlib/tools.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL3Rvb2xzLnB5) | `78.4% <100%> (ø)` | |
| [pandas/core/groupby/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9nZW5lcmljLnB5) | `89.08% <100%> (ø)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.61% <100%> (ø)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97% <100%> (-0.12%)` | :arrow_down: |
| [pandas/plotting/\_matplotlib/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19tYXRwbG90bGliL19faW5pdF9fLnB5) | `100% <100%> (ø)` | |
| ... and [18 more](https://codecov.io/gh/pandas-dev/pandas/pull/26414/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26414?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/26414?src=pr&el=footer). Last update [5b10e31...ceedd3b](https://codecov.io/gh/pandas-dev/pandas/pull/26414?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

I am too :zzz: right now to have much sensible comments on the code.

If you are interested in pulling this whole thing out and putting it in `matplotlib/matplotlib-pandas` I am  open to that and would happily make sure anyone from the pandas side who needs / wants access will have it.  That is probably something to look at down the road after sorting out what you want the interface to be (mostly to avoid obligatory coordinated releases).

If there is interest in more aggressive re-thinking of the API https://github.com/DistrictDataLabs/yellowbrick/issues/826 , https://github.com/matplotlib/matplotlib/pull/14058, and https://paper.dropbox.com/doc/Matplotlib-4.0--AdQMb0ugRPWeqq3U4dEAHuSDAQ-WTYwd0NQaSHTjtLUZwkNx may be of interest.
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-06-08 22:28:02 UTC
100% agree, there is no functional change, and no public API change in this PR. Just moving all the code specific to matplotlib from `pandas/plotting/*` to `pandas/plotting/_matplotlib/*`. All the calls to the "public" functions in `_matplotlib` are done by importing the module in a lazy way (that fails when matplotlib is not installed in the same was as it fails now).

Ideally would like to avoid touching any tests to guarantee that no unintentional functional change happens. In practice I need to update the import paths of some tests, since they import private functions that have been moved.

Still work in progress, the change is much bigger than what I originally thought, but will provide more details on what the PR involves when it's ready to be merged.
thanks @datapythonista yeah also happy to do this in pieces if its easier.
This should be ready now. This PR just moves code around, and shouldn't change the behavior of pandas at all. The implemented changes are:

- All the code that imports and uses matplotlib now lives in `pandas/plotting/_matplotlib`
- There is a clear API for what's in `pandas.plotting` and also in `pandas.plotting._matplotlib`, and no private methods are imported (except for tests, benchmarks and deprecated code)
- The changes in this PR should make very easy to change the matplotlib plotting backend by another one based in bokeh, holoviews, altair... as far as they are implemented using the same public API

Once this is merged I'll open issues to discuss the possible follow up changes:
- Using an option to specify the plotting backend
- Possible deprecations (there are up to 4 ways to plot the same thing in pandas, e.g. `df.plot(kind='box')`, `df.plot.box()`, `df.boxplot()` and `pandas.plotting.boxplot(df)`). Probably worth deprecating some, making the API for plotting backends simpler, and making users life easier by providing a single consistent way
- Possibly moving the matplotlib backend out of the pandas repo.
- Whether the backends should implement plots like `andrews_curves`, `autocorrelation_plot`,... or if they shouldn't be part of the pandas plotting API (but a third party).
- ...
Thanks for the review, addressed comments. Agree on splitting the tests, will follow up with that. I thought using relative imports made the matplotlib backend more decoupled (and easy to move if that's wanted at some point). Changed them to absolute imports for now, and we can see in the future if that's needed/useful.
lgtm. @jorisvandenbossche or @TomAugspurger 
This seems to have lost our warning

```
In [1]: paste
import pandas as pd
import matplotlib.pyplot as plt
df = pd.Series(1, pd.date_range('2000', periods=12))

fig, ax = plt.subplots()

ax.plot(df.index, df.values)
## -- End pasted text --
/Users/taugspurger/sandbox/pandas/pandas/plotting/_converter.py:127: FutureWarning: Using an implicitly registered datetime converter for a matplotlib plotting method. The converter was registered by pandas on import. Future versions of pandas will require you to explicitly register matplotlib converters.

To register the converters:
        >>> from pandas.plotting import register_matplotlib_converters
        >>> register_matplotlib_converters()
  warnings.warn(msg, FutureWarning)
Out[1]: [<matplotlib.lines.Line2D at 0x10c614048>]
```

There's no warning on this branch.

I think that's because `import pandas` doesn't do an `import matplotlib` inside a try / except anymore. On master, `import pandas` would register our units / converters.

I don't recall when that deprecation is slated for removal. May as well be 0.25 though?
On the converter warning, that was added in 0.21. https://github.com/pandas-dev/pandas/pull/24964. Something happened, and it wasn't actually showing up till 0.24. So I'm not sure what's best here.

If you do want to enforce the deprecation, then I'd recommend merging this first, and cleaning up the deprecation in a followup.
@datapythonista can you merge master.

Conflicting files
pandas/plotting/_compat.py
@TomAugspurger I restored the warning for now. The problem is that the `pandas.plotting._matplotlib` module is never loaded with the example in your comment, so registering the converters in that module, where it should happen is not possible.

I implemented it in a somehow hacky way. In `pandas.plotting` I test whether matplotlib is installed, and if it is, I register the converters (and load `pandas.plotting._matplotlib`). This is not ideal, but we'll still be able to use other plotting backends without problems, and it's temporary (we can see later if we want to remove the automatic registration of converters in 0.25, 1.0 or when).

I think this solution is the best, let me know if you have a better idea (I'm not sure if this is what you were proposing).
can you merge master
>  I restored the warning for now.

Thanks. Looks good, and I'll do a followup issue to discuss removing it (I think after 0.25).
> This should be ready now. This PR just moves code around, and shouldn't change the behavior of pandas at all. The implemented changes are:
> 
> * All the code that imports and uses matplotlib now lives in `pandas/plotting/_matplotlib`
> * There is a clear API for what's in `pandas.plotting` and also in `pandas.plotting._matplotlib`, and no private methods are imported (except for tests, benchmarks and deprecated code)
> * The changes in this PR should make very easy to change the matplotlib plotting backend by another one based in bokeh, holoviews, altair... as far as they are implemented using the same public API
> 
> Once this is merged I'll open issues to discuss the possible follow up changes:
> 
> * Using an option to specify the plotting backend
> * Possible deprecations (there are up to 4 ways to plot the same thing in pandas, e.g. `df.plot(kind='box')`, `df.plot.box()`, `df.boxplot()` and `pandas.plotting.boxplot(df)`). Probably worth deprecating some, making the API for plotting backends simpler, and making users life easier by providing a single consistent way
> * Possibly moving the matplotlib backend out of the pandas repo.
> * Whether the backends should implement plots like `andrews_curves`, `autocorrelation_plot`,... or if they shouldn't be part of the pandas plotting API (but a third party).
> * ...

Hi, thanks for working on this issue. If there is native support for a plotting backend switch, implementing new plotting solutions for Pandas will be much more attractive! 

Does every backend need to provide a this APIs or are they optional? This could, in my opinion) discourage people to implement their backend solution + restrict the creativity of the developers because each backend could provide some innovative ideas (eg matplotlib vs seaborn or bokeh vs holoviews).

Furthermore, is there also a minimal set of keyword arguments (x, y, color, xlabel, ...) that has to be implemented for each glyph such that some minimal examples work for each backend? Also, will the development of the plotting backends happen inside pandas-dev, a new pandas-plotting-dev or a own repository like hvplot/Pandas-Bokeh?

Best, Patrik
Backends should implement the same API, aside from the return type. For alternative APIs, libraries can accept DataFrames, just like seaborn.

> On Jun 2, 2019, at 21:31, Patrik Hlobil <notifications@github.com> wrote:
> 
> This should be ready now. This PR just moves code around, and shouldn't change the behavior of pandas at all. The implemented changes are:
> 
> All the code that imports and uses matplotlib now lives in pandas/plotting/_matplotlib
> There is a clear API for what's in pandas.plotting and also in pandas.plotting._matplotlib, and no private methods are imported (except for tests, benchmarks and deprecated code)
> The changes in this PR should make very easy to change the matplotlib plotting backend by another one based in bokeh, holoviews, altair... as far as they are implemented using the same public API
> Once this is merged I'll open issues to discuss the possible follow up changes:
> 
> Using an option to specify the plotting backend
> Possible deprecations (there are up to 4 ways to plot the same thing in pandas, e.g. df.plot(kind='box'), df.plot.box(), df.boxplot() and pandas.plotting.boxplot(df)). Probably worth deprecating some, making the API for plotting backends simpler, and making users life easier by providing a single consistent way
> Possibly moving the matplotlib backend out of the pandas repo.
> Whether the backends should implement plots like andrews_curves, autocorrelation_plot,... or if they shouldn't be part of the pandas plotting API (but a third party).
> ...
> Hi, thanks for working on this issue. If there is native support for a plotting backend switch, implementing new plotting solutions for Pandas will be much more attractive!
> 
> Does every backend need to provide a this APIs or are they optional? This could, in my opinion) discourage people to implement their backend solution + restrict the creativity of the developers because each backend could provide some innovative ideas (eg matplotlib vs seaborn or bokeh vs holoviews).
> 
> Furthermore, is there also a minimal set of keyword arguments (x, y, color, xlabel, ...) that has to be implemented for each glyph such that some minimal examples work for each backend? Also, will the development of the plotting backends happen inside pandas-dev, a new pandas-plotting-dev or a own repository like hvplot/Pandas-Bokeh?
> 
> Best, Patrik
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

@datapythonista are you able to reproduce the CI failures like https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=12231 locally? If not, I can try taking a look.
No, I'm taking a look, but very confused on what's going on, even the mypy error. The CI was green before merging master, which confuses me even more. If you can take a look too, that would be great, let me know if you find something.
@datapythonista haven't had the chance to look too deep but interesting that `boxplot_frame` throws a mypy error while `hist_frame` the line above doesn't. What's different between those two is the Appender - can you see if removing that maybe silences the mypy error? 
thanks @WillAyd, I also saw that, and removing the Appender does fix the problem, but I think there is a root error, Appender is being used eveywhere... :\
Yea not sure why it's appearing now. Probably just need to annotate Appender for proper inference.

I don't think it's critical for this PR so if that's the only hangup feel free to add `type: ignore` and we can address in a follow up
I tried annotating `Appender` but didn't work. I saw a mypy issue where this error was raised for unrelated problems. One of the test failures (happens just with couple of builds) is related to boxplot too. I'll see if I can find the problem for that, as it's difficult to debug the mypy error. With some luck all the problems are caused by the same. Thanks for the help.
No problem. As mentioned I think OK to mark as type: ignore for now so it doesn't hold this up. I've opened #26629 to address separately