Skip to content
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

Matplotlib 3.3 compatibility fixups #35393

Merged
merged 11 commits into from Jul 23, 2020
Merged

Conversation

TomAugspurger
Copy link
Contributor

@TomAugspurger TomAugspurger commented Jul 23, 2020

Closes #34850
Closes #35350

Haven't tested with matplotlib's older than 3.2 yet. We'll see what CI says.

@TomAugspurger TomAugspurger added Visualization plotting Compat pandas objects compatability with Numpy or Python functions labels Jul 23, 2020
@TomAugspurger TomAugspurger added this to the 1.1 milestone Jul 23, 2020
@@ -299,6 +300,11 @@ def plot_group(keys, values, ax):
if fontsize is not None:
ax.tick_params(axis="both", labelsize=fontsize)
if kwds.get("vert", 1):
ticks = ax.get_xticks()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from a change in MPL being stricter about set_xticks. This worked for matplotlib 3.2

import pandas as pd
import matplotlib.pyplot as plt
import string
import numpy as np

fig, (ax1, ax2) = plt.subplots(ncols=2, sharex=True)
ax1.boxplot([
    np.array([1, 2, 3, 4]),
    np.array([1, 2, 3, 4])
])
ax2.boxplot([
    np.array([1, 2, 2, 3]),
    np.array([1, 2, 3, 4])
])

ax1.set_xticklabels(['A', 'B'])

We set 2 x ticklabels despite there being 4.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was a deliberate change... matplotlib/matplotlib#17266 The obvious problem if you only give us 2 tick labels is that its ambiguous which of the 4 ticks you wanted labeled.

MPL now has categorical axes which I think were added because pandas has them. Probably too big a project to homogenize, but....

pandas/plotting/_matplotlib/converter.py Show resolved Hide resolved
@@ -411,8 +391,8 @@ def __call__(self):
interval = self._get_interval()
freq = f"{interval}L"
tz = self.tz.tzname(None)
st = _from_ordinal(dates.date2num(dmin)) # strip tz
ed = _from_ordinal(dates.date2num(dmax))
st = dmin.replace(tzinfo=None)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the comment is correct that this is really about stripping the tz, then this is cleaner since it doesn't rely on matplotlib's Datetime <-> numeric conversion.

@@ -331,7 +332,7 @@ def test_freq_with_no_period_alias(self):
bts = tm.makeTimeSeries(5).asfreq(freq)
_, ax = self.plt.subplots()
bts.plot(ax=ax)
assert ax.get_lines()[0].get_xydata()[0, 0] == bts.index[0].toordinal()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test, and some similar, are invalid now. The relationship between the plotted xy data and ordinals isn't necessarily stable / valuable.

for kind in kinds:

_, ax = self.plt.subplots()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some strange interaction / issue when plotting on existing axes. IIRC it's from MPL being stricter about things. I didn't investigate too much.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its preferable to keep the reference to the figure and axes and then reuse, rather than depend on the implicit ability of plt.subplots() to pick up the same axes...

@TomAugspurger
Copy link
Contributor Author

cc @jklymak. The only change I'm uncertain about being a bug vs. deliberate is #35393 (comment).

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. do we need to bump mpl min? (I am not sure what the oldest we test)

pandas/tests/plotting/test_converter.py Outdated Show resolved Hide resolved
@jklymak
Copy link
Contributor

jklymak commented Jul 23, 2020

do we need to bump mpl min? (I am not sure what the oldest we test)

Matplotlib has supported datetime64 since v2.2.0, though there have been some bug fixed along the way....

@TomAugspurger
Copy link
Contributor Author

2.2.0 is our minimum right now, so I think we're good.

@jreback jreback merged commit 00ea10c into pandas-dev:master Jul 23, 2020
@jreback
Copy link
Contributor

jreback commented Jul 23, 2020

thanks @TomAugspurger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compat pandas objects compatability with Numpy or Python functions Visualization plotting
Projects
None yet
3 participants