New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate automatically registering matplotlib units (partial revert of 0.21.0) #18301

Closed
jorisvandenbossche opened this Issue Nov 15, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@jorisvandenbossche
Member

jorisvandenbossche commented Nov 15, 2017

Given the feedback we have got (#18153, #18192, #18212, #18283, matplotlib/matplotlib#9577, matplotlib/matplotlib#9610, matplotlib/matplotlib#9771, pydata/xarray#166), it seems we underestimated the impact and we should consider (partly) reverting this change to properly deprecate it instead.

This should also give matplotlib the time to implement basic datetime64 support (matplotlib/matplotlib#9610, matplotlib/matplotlib#9779)

Depending on how we can do this, this might have the consequence that have to undo temporarily the lazy import of matplotlib (affecting the import time). See also discussion in #18283

Opening this issue to keep track of it, should be decided/done for 0.21.1

@TomAugspurger

This comment has been minimized.

Show comment
Hide comment
@TomAugspurger

TomAugspurger Nov 15, 2017

Contributor

Yeah, we should add back the converter registration on pandas import.

I have a branch started that should be able to emit a warning when people rely on the implicit registration, so we'll be able to deprecate this cleanly.

Contributor

TomAugspurger commented Nov 15, 2017

Yeah, we should add back the converter registration on pandas import.

I have a branch started that should be able to emit a warning when people rely on the implicit registration, so we'll be able to deprecate this cleanly.

@jorisvandenbossche

This comment has been minimized.

Show comment
Hide comment
@jorisvandenbossche

jorisvandenbossche Nov 15, 2017

Member

And then doing the registering manually will overwrite the deprecated units as a way to avoid the warning? And also using pandas plotting functionality the first time will do the same?

Side question: now we recommend people to do from pandas.tseries import converter, but IMO it would be more logical to have this in pandas.plotting ? We can't remove (or even deprecate will be annoying) the tseries one (certainly to support multiple pandas versions), but maybe we could consider moving it to pandas.plotting as well and start using that in our docs?

Member

jorisvandenbossche commented Nov 15, 2017

And then doing the registering manually will overwrite the deprecated units as a way to avoid the warning? And also using pandas plotting functionality the first time will do the same?

Side question: now we recommend people to do from pandas.tseries import converter, but IMO it would be more logical to have this in pandas.plotting ? We can't remove (or even deprecate will be annoying) the tseries one (certainly to support multiple pandas versions), but maybe we could consider moving it to pandas.plotting as well and start using that in our docs?

@TomAugspurger

This comment has been minimized.

Show comment
Hide comment
@TomAugspurger

TomAugspurger Nov 15, 2017

Contributor

Here's the behavior:

In [1]: import pandas as pd
imp
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]: ax.plot(s)
/Users/taugspurger/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/plotting/_converter.py:77: 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.tseries import converter
        >>> converter.register()
  warnings.warn(msg, FutureWarning)
Out[5]: [<matplotlib.lines.Line2D at 0x10e18eb38>]

vs.

In [1]: import pandas as pd

In [2]: import matplotlib.pyplot as plt

In [3]: from pandas.tseries import converter

In [4]: converter.register()

In [5]: fig, ax = plt.subplots()

In [6]: s = pd.Series(range(12), index=pd.date_range('2017', periods=12))

In [7]: ax.plot(s)
Out[7]: [<matplotlib.lines.Line2D at 0x1097c76d8>]

(the implementation isn't the prettiest, but it works).

Happy to move the recommended import location (while keeping pandas.tseries there for compat).

Contributor

TomAugspurger commented Nov 15, 2017

Here's the behavior:

In [1]: import pandas as pd
imp
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]: ax.plot(s)
/Users/taugspurger/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/plotting/_converter.py:77: 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.tseries import converter
        >>> converter.register()
  warnings.warn(msg, FutureWarning)
Out[5]: [<matplotlib.lines.Line2D at 0x10e18eb38>]

vs.

In [1]: import pandas as pd

In [2]: import matplotlib.pyplot as plt

In [3]: from pandas.tseries import converter

In [4]: converter.register()

In [5]: fig, ax = plt.subplots()

In [6]: s = pd.Series(range(12), index=pd.date_range('2017', periods=12))

In [7]: ax.plot(s)
Out[7]: [<matplotlib.lines.Line2D at 0x1097c76d8>]

(the implementation isn't the prettiest, but it works).

Happy to move the recommended import location (while keeping pandas.tseries there for compat).

@jorisvandenbossche

This comment has been minimized.

Show comment
Hide comment
@jorisvandenbossche

jorisvandenbossche Nov 15, 2017

Member

that looks good!

Member

jorisvandenbossche commented Nov 15, 2017

that looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment