API: Restore implicit converter registration
State: closed

Closes #18301 

TODO:

- [ ] find best home for the import
- [ ] update deprecation message

Labels:
name: Visualization
description: plotting
name: API Design

Comments:
I added the "special" section in the whatsnew for this and the changes we're making to NaN/empty sum.
What are people's thoughts on moving the `register` import to `pandas.plotting` instead of `pandas.tseries`? I'm not sure I see the benefit, unless we're planning to remove `pandas.tseries` in the future.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=h1) Report
> Merging [#18307](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/9c799e2c4331e5e42cfda03323eef165feb5be1a?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `84.61%`.

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

```diff
@@            Coverage Diff             @@
##           master   #18307      +/-   ##
==========================================
- Coverage   91.39%   91.38%   -0.02%     
==========================================
  Files         164      164              
  Lines       49854    49869      +15     
==========================================
+ Hits        45566    45571       +5     
- Misses       4288     4298      +10
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.18% <84.61%> (ø)` | :arrow_up: |
| #single | `39.45% <30.76%> (-0.05%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `82.37% <66.66%> (-0.09%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.92% <94.11%> (+0.67%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.8% <0%> (-0.1%)` | :arrow_down: |
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `100% <0%> (+100%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18307?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/18307?src=pr&el=footer). Last update [9c799e2...ba88a04](https://codecov.io/gh/pandas-dev/pandas/pull/18307?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/18307?src=pr&el=h1) Report
> Merging [#18307](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/7627ccaf9442f4101afda69b6077e7f035e23543?src=pr&el=desc) will **increase** coverage by `0.08%`.
> The diff coverage is `92.06%`.

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

```diff
@@            Coverage Diff             @@
##           master   #18307      +/-   ##
==========================================
+ Coverage   91.35%   91.43%   +0.08%     
==========================================
  Files         164      157       -7     
  Lines       49802    51424    +1622     
==========================================
+ Hits        45496    47019    +1523     
- Misses       4306     4405      +99
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.3% <92.06%> (+0.16%)` | :arrow_up: |
| #single | `40.58% <42.85%> (-0.3%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18307?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `100% <100%> (+100%)` | :arrow_up: |
| [pandas/core/config\_init.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29uZmlnX2luaXQucHk=) | `98.46% <100%> (+0.11%)` | :arrow_up: |
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `82.37% <66.66%> (-0.13%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `66.52% <97.14%> (+1.26%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `81.6% <0%> (-18.4%)` | :arrow_down: |
| [pandas/io/json/json.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vanNvbi5weQ==) | `91.75% <0%> (-8.25%)` | :arrow_down: |
| [pandas/io/formats/style.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvc3R5bGUucHk=) | `96.21% <0%> (-3.79%)` | :arrow_down: |
| [pandas/core/indexes/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9pbnRlcnZhbC5weQ==) | `93.05% <0%> (-0.17%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.81% <0%> (-0.1%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/pandas-dev/pandas/pull/18307/diff?src=pr&el=tree-more) | |

------

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

Should we also add some language on how to avoid this warning *without* registering the converters by default? So if you in the future don't want to manually register them, but rely on the matplotlib ones (eg when working with datetime.datetime) ?
I guess a pandas option?

```
pd.options.register_formatters : bool
```

And we check that before ever calling register?
Though, future-proofing a bit, should that be spread across several options, one per type? We currently register for

```
    units.registry[lib.Timestamp] = DatetimeConverter()
    units.registry[Period] = PeriodConverter()
    units.registry[pydt.datetime] = DatetimeConverter()
    units.registry[pydt.date] = DatetimeConverter()
    units.registry[pydt.time] = TimeConverter()
    units.registry[np.datetime64] = DatetimeConverter()
```

In the ideal future, we would I think just do Timestamp and Period, and matplotlib would have datetime64, I don't know who wants to own python dates / times / datetimes.

cc @tacaswell if any of this interests you.
Added a rough implementation of the option in 16c33f1.


```python
In [1]: import pandas as pd

In [2]: import matplotlib.pyplot as plt

In [3]: fig, ax = plt.subplots()

In [4]: s = pd.Series(range(12), index=pd.date_range('2017', periods=12))

In [5]: pd.options.plotting.matplotlib.register_formatters = False

In [6]: ax.plot(s)
Out[6]: [<matplotlib.lines.Line2D at 0x1131ce588>]
```
> What are people's thoughts on moving the register import to pandas.plotting instead of pandas.tseries? I'm not sure I see the benefit, unless we're planning to remove pandas.tseries in the future.

if its not too much trouble. I would like to at least deprecate ``pandas.tseries``
Thanks :heart: .
Some other things related to the option:

- It seems you kept the terminology of 'converters' (and not 'formatters') throughout the whatsnew and code? (which is I think the terminology of matplotlib, as the converter calls a formatter). But the option is called 'register_formatters'. So we should choose here.

- When you set the `pd.options.plotting.matplotlib.register_formatters = False`, they will also not get registered when using pandas' plot methods ? I don't think that is the case, but shouldn't it be?

- The "unregistering" (setting `pd.options.plotting.matplotlib.register_formatters = False`) will also remove the converters for datetime.datetime, which are actually the default ones. Ideally they are set back as well. @tacaswell Is there a way to restore the default units? (like with rcParams). Of course we can also save them when we overwrite them, to set it back properly.

- I would make the option maybe a bit shorter, like `pd.options.plotting.register_matplotlib_formatters` (not shorter, but a '.' less so quicker to tab-complete) ?
> It seems you kept the terminology of 'converters' (and not 'formatters') 

Switched to formatters everywhere.

> they will also not get registered when using pandas' plot methods ?

Correct. To be more specific, they're *always* registered when pandas is imported. The option toggles whether or not they're in the `matplotlib.units.registry` dict. `True` adds them, `False` removes them. That means that doing

```python
In [8]: pd.options.plotting.matplotlib.register_formatters = False

In [9]: s.plot()
```

Will fall back to whatever matplotlib does with datetimes / periods (potentially an exception, e.g. for periods).

> will also remove the converters for datetime.datetime, which are actually the default ones

Hmm, not sure about that. My registry is empty upon import:

```python
In [1]: import matplotlib.units

In [2]: matplotlib.units.registry
Out[2]: {}
```

Though maybe I'm misunderstanding. The intent is return the registry to the state before we mucked with it.

> I would make the option maybe a bit shorter

Yeah, thoughts on `pd.options.plotting.mpl.converters`? I want the `.mpl` / matplotlib namespace for if / when we allow different engines for `.plot`.
The unit registry gets filled on import.  If you import `matplotlib.dates` they will be added.  These imports are done in `pyplot` so _most_ users have this import done. 
Ah thanks.

```python
In [1]: import matplotlib.units

In [2]: import matplotlib.pyplot as plt

In [3]: matplotlib.units.registry
Out[3]:
{str: <matplotlib.category.StrCategoryConverter at 0x10beb7f60>,
 numpy.str_: <matplotlib.category.StrCategoryConverter at 0x10beb7f28>,
 bytes: <matplotlib.category.StrCategoryConverter at 0x10beb7ef0>,
 numpy.bytes_: <matplotlib.category.StrCategoryConverter at 0x10beb7f98>,
 datetime.date: <matplotlib.dates.DateConverter at 0x10bf2c208>,
 datetime.datetime: <matplotlib.dates.DateConverter at 0x10bf2c240>}

In [4]: import pandas as pd

In [5]: matplotlib.units.registry
Out[5]:
{str: <matplotlib.category.StrCategoryConverter at 0x10beb7f60>,
 numpy.str_: <matplotlib.category.StrCategoryConverter at 0x10beb7f28>,
 bytes: <matplotlib.category.StrCategoryConverter at 0x10beb7ef0>,
 numpy.bytes_: <matplotlib.category.StrCategoryConverter at 0x10beb7f98>,
 datetime.date: <pandas.plotting._converter.DatetimeConverter at 0x10bf2c240>,
 datetime.datetime: <pandas.plotting._converter.DatetimeConverter at 0x10eaa3828>,
 pandas._libs.tslib.Timestamp: <pandas.plotting._converter.DatetimeConverter at 0x10eaa3780>,
 pandas._libs.period.Period: <pandas.plotting._converter.PeriodConverter at 0x10eaa37b8>,
 datetime.time: <pandas.plotting._converter.TimeConverter at 0x10bf2c208>,
 numpy.datetime64: <pandas.plotting._converter.DatetimeConverter at 0x10eaac0f0>}

In [6]: pd.options.plotting.matplotlib.register_formatters = False

In [7]: matplotlib.units.registry
Out[7]:
{str: <matplotlib.category.StrCategoryConverter at 0x10beb7f60>,
 numpy.str_: <matplotlib.category.StrCategoryConverter at 0x10beb7f28>,
 bytes: <matplotlib.category.StrCategoryConverter at 0x10beb7ef0>,
 numpy.bytes_: <matplotlib.category.StrCategoryConverter at 0x10beb7f98>}
```

So not great... We could of course maintain our own global state when we overwrite them... Though we'll have to make sure `matplotlib.pyplot` is imported first. Let me give that a shot.
Just pushed a few changes, if you have a chance to review again.

- Moved recommended import to `pandas.plotting.register_formatters`, `pandas.plotting.deregister_formatters`
- Added a `deregister_formatters` to undo what pandas does (needed it anyway for the callback, might as well make it public)
- Updated the release note / error message to always use the term "converter"


TODO:

- [x] add depr warning in tseries
- [ ] *Another* PR to matplotlib updating their FAQ (sorry)

Should be able to do those later today.
Hello @TomAugspurger! Thanks for updating the PR.















Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on December 01, 2017 at 14:53 Hours UTC
@TomAugspurger Thanks a lot for the updates!

> Moved recommended import to pandas.plotting.register_formatters, pandas.plotting.deregister_formatters

+1. I was only wondering if we should make it more explicit it is for matplotlib (like `register_matplotlib_converters`)? (although I know I previously wondered if it could be shorter ... :-), but for case matplotlib is not the only supported plotting engine in the future)

> Yeah, thoughts on pd.options.plotting.mpl.converters? I want the .mpl / matplotlib namespace for if / when we allow different engines for .plot.

OK, yes, having 'matplotlib' in there makes sense. Although I prefer the full 'matplotlib' instead of `mpl`. And I think I find 'register_converters = True/False' clearer than `converters = True/False`. Although then we are back at the original `pd.options.plotting.matplotlib.register_converters` :-)
Something else: I was wondering if it actually *would* be possible to do the registration/deregistration in a context manager in our plotting machinery. The reason we said this would be impossible is that you after a `df.plot` call you want to use matplotlib's machinery to tune the figure (eg `ax.set_xlim(..)`), so this still needs to know about the converters. 
But, it seems that the `ax` object actually stores the used converters for plotting. So maybe that would be enough?

```
In [15]: s = pd.Series(range(10), pd.date_range("2012-01-01", periods=10))

In [16]: ax = s.plot()

In [17]: ax.xaxis.converter
Out[17]: <pandas.plotting._converter.PeriodConverter at 0x7f3c112f9ac8>
```
As a small example:

```
In [1]: s = pd.Series(range(10), pd.date_range("2012-01-01", periods=10))

In [3]: import matplotlib.pyplot as plt

In [4]: fig, ax = plt.subplots()

In [5]: s.plot(ax=ax)
Out[5]: <matplotlib.axes._subplots.AxesSubplot at 0x7fd4580d4fd0>

In [6]: pd.plotting.deregister_converters()

In [7]: ax.xaxis.converter
Out[7]: <pandas.plotting._converter.PeriodConverter at 0x7fd45d4538d0>

In [8]: ax.set_xlim("2012-01-01", "2012-01-04")    # <---- still works although converters are no longer registered
Out[8]: (15340, 15343)

In [10]: import matplotlib

In [11]: matplotlib.units.registry
Out[11]: 
{bytes: <matplotlib.category.StrCategoryConverter at 0x7fd472ac6278>,
 datetime.date: <matplotlib.dates.DateConverter at 0x7fd45d1fd6d8>,
 numpy.bytes_: <matplotlib.category.StrCategoryConverter at 0x7fd472ac6320>,
 str: <matplotlib.category.StrCategoryConverter at 0x7fd472ac62e8>,
 numpy.str_: <matplotlib.category.StrCategoryConverter at 0x7fd472ac62b0>,
 datetime.datetime: <matplotlib.dates.DateConverter at 0x7fd45d1fd710>}
```

@tacaswell any idea there would be caveats to this approach? Are there customizations that you can do on plots that will typically not work like that? (that would not be using the stored `converter` on the axis method)
Oh, that's neat.

But, do we want to do that for this PR or later on? And what would that look like? How about the option takes three values:

- True  (register right away)
- False (don't register / unregister)
- auto (Use a context manager in our plots)

The current default will be True which is compatible with <0.21.0. In the future it'll be 'auto'.

Though I'm not sure I'll have time to implement the 'auto' behavior today, and I'll be traveling the rest of the week.
> But, do we want to do that for this PR or later on?

I don't think it needs to be in this PR (as this already introduces a lot of useful architecture), but if we want to do it, I think we should try to do it for 0.21.1, as it will again be a change in behaviour (the change from True (once plotted once) to auto).

>  And what would that look like? How about the option takes three values:

I think those three options are good: either registerer it matplotlib-wide, either never registerer it, or only register it temporarily for plots that are made with pandas.

But we will probably still need to look at a couple of edge cases if it actually works.

I would say let's try to merge this PR for now, and leave the 'auto' behaviour for a follow-up. I could also try to look at it this week.

Another thing I was thinking: for users that were just relying on the converter by importing pandas (but for the rest plotting with pure matplotlib), deprecating it now can be a bit annoying, as the proper support in matplotlib will only come in the next release. 
So in practice that means that they will have to add temporarily `pd.plotting.register_matplotlib_converters()` to their code for a couple of months, instead of directly being able to switch to the new matplotlib support.

So that said, another option is to just undo the change now for 0.21.1 and keep the actual deprecation for 0.22.0 (assuming that will only be released when matplotlib 2.2.0 will also more or less be released). 
@tacaswell Is there a timeline for matplotlib 2.2.0? Or could the datetime64 converter be sneaked into 2.1.1? (considering it as kind of a 'regression fix' to deal with the changes in pandas ..)

(just a thought, not sure yet if I find postponing actual deprecation a good idea myself)
@TomAugspurger What do you think of the idea above to completely revert this for 0.21.1, and target this PR for 0.22.0, in the idea that by then matplotlib has the datetime64 support, and many users won't need to do the manual registration?
had a talk with @tacaswell about this. are we sure we want this behavior?
What do you mean with "this behavior"?

Ideally (for pandas), matplotlib would already include the datetime64 converters in 2.1.1 bugfix release (the merged PR there was now targetted for matplotlib 2.2). But I assume they are not favoring that since it not a bugfix from the matplotlib point of view (if you see @tacaswell you could bring that up)
> Ideally (for pandas), matplotlib would already include the datetime64 converters in 2.1.1 bugfix release (the merged PR there was now targetted for matplotlib 2.2). But I assume they are not favoring that since it not a bugfix from the matplotlib point of view (if you see @tacaswell you could bring that up)

yes, we changed and let's move on. mpl will catch up with the fix. reverting just makes this even more confusing.
This PR *is* already reverting it (and we agree on doing this I thought), the question is more to already raise a deprecation warning or not?
Just thinking out loud here to make sure I understand:

1. We're going to register the converters again on `import pandas` either way
2. We're uncertain on whether to include the deprecation warning for using an implicitly registered converter for datetime data.

The uncertainty with 2 comes from us *maybe* deprecating `converter.register`. We don't want to say "you're required to register this" and then remove that shortly after, right?

I think pandas will always have to have *some* converters, if just for things like `PeriodIndex`, which matplotlib has no knowledge of. So I think my vote is for including the deprecation warning.
One more issue...  If someone with mpl2.2+ (with their datetime converters), we should *not* warn when they plot datetimes and have imported pandas, but have not registered the converters. I think...

Actually, we should just not override their formatter maybe.
> The uncertainty with 2 comes from us maybe deprecating converter.register. We don't want to say "you're required to register this" and then remove that shortly after, right?

No, my reason to *delay* deprecation is that for the majority of the users (that do like `plt.plot(s.index, s.values` with DatetimeIndex) the matplotlib converters will be sufficient (I think). 
For this part, if we release 0.21.1 with deprecation warning, we ask them to manually register the converters. But once matplotlib 2.2 comes out somewhere next year, they don't actually need this manually registering of the pandas converters anymore.  
If we wait with the deprecation until pandas 0.22.0 (assuming that matplotlib 2.2 is released by then), they will never need to bother with registering the converters

> I think pandas will always have to have some converters, if just for things like PeriodIndex, which matplotlib has no knowledge of. So I think my vote is for including the deprecation warning.

yes, and for those we will need to do that. But I think the majority of the usecase is DatetimeIndex and not PeriodIndex.