-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Matplotlib 3.3 compatibility fixups #35393
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
Conversation
@@ -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() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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....
@@ -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) |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
cc @jklymak. The only change I'm uncertain about being a bug vs. deliberate is #35393 (comment). |
There was a problem hiding this 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)
Matplotlib has supported datetime64 since v2.2.0, though there have been some bug fixed along the way.... |
2.2.0 is our minimum right now, so I think we're good. |
thanks @TomAugspurger |
This locator takes effect when a datetime x-range axis is < 5 seconds (among a few other conditions), and the typo causes only one xtick at the very left to be displayed.
Typo fix in MilliSecondLocator for #35393 This locator takes effect when a datetime x-range axis is < 5 seconds (among a few other conditions), and the typo causes only one xtick at the very left to be displayed.
Closes #34850
Closes #35350
Haven't tested with matplotlib's older than 3.2 yet. We'll see what CI says.