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

COMPAT: mpl 3.0 #22870

Merged
merged 2 commits into from
Sep 28, 2018
Merged

COMPAT: mpl 3.0 #22870

merged 2 commits into from
Sep 28, 2018

Conversation

TomAugspurger
Copy link
Contributor

Closes #22790

@pep8speaks
Copy link

Hello @TomAugspurger! Thanks for submitting the PR.

@TomAugspurger
Copy link
Contributor Author

The test pandas/tests/plotting/test_datetimelike.py::TestTSPlot::test_high_freq takes 123s for me locally. That seems excessive. I wonder if there was a performance regression there, or whether we can make the test smaller.

@TomAugspurger
Copy link
Contributor Author

5fbffe4 was the original commit (6 years ago). Nothing in that looks like it depends on the number of points, just the freq, so I'm going to reduce the number to 100 (finishes in 0.5s locally with that).

@TomAugspurger TomAugspurger added Visualization plotting CI Continuous Integration labels Sep 28, 2018
@TomAugspurger TomAugspurger added this to the 0.24.0 milestone Sep 28, 2018
img = ax.collections[0]
cbar = self.fig.colorbar(img, ax=ax, **kwds)

if _mpl_ge_3_0_0():
# The workaround below is no longer necessary.
Copy link
Contributor

Choose a reason for hiding this comment

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

could xfail?

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 is in the plotting code itself, not in the test. The workaround is necessary for mpl < 3.0, but breaks things in mpl >= 3.0

@TomAugspurger
Copy link
Contributor Author

@jreback good to merge on green, to get master passing again? (aside from #22871 which is failing our strict warnings build)

@jreback
Copy link
Contributor

jreback commented Sep 28, 2018

yep

@codecov
Copy link

codecov bot commented Sep 28, 2018

Codecov Report

Merging #22870 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #22870      +/-   ##
==========================================
- Coverage   92.19%   92.18%   -0.01%     
==========================================
  Files         169      169              
  Lines       50827    50830       +3     
==========================================
  Hits        46860    46860              
- Misses       3967     3970       +3
Flag Coverage Δ
#multiple 90.6% <100%> (-0.01%) ⬇️
#single 42.37% <33.33%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/plotting/_compat.py 91.3% <100%> (+0.39%) ⬆️
pandas/plotting/_core.py 83.35% <100%> (-0.2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d115900...2ca4963. Read the comment docs.

@TomAugspurger TomAugspurger merged commit e45a6c1 into pandas-dev:master Sep 28, 2018
@TomAugspurger TomAugspurger deleted the mpl-compat branch September 28, 2018 15:06
@gfyoung
Copy link
Member

gfyoung commented Sep 28, 2018

Given that 0.23.x is still active, I would recommend backporting CI fixes like this. When I attempted to backport an actual regression last time, I ran into numerous CI-related issues unrelated to the original fix.

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018
* COMPAT: mpl 3.0

* faster test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Visualization plotting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants