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

Bump minimum Matplotlib version to 3.5 #2258

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Oct 4, 2023

Rationale

As agreed in #2252. Hopefully the minimum version tests are now no more broken than the others.

Implications

@rcomer rcomer marked this pull request as draft October 4, 2023 17:16
@@ -497,7 +498,10 @@ def test_gridliner_title_adjust():
plt.rcParams['axes.titley'] = None

fig = plt.figure(layout='constrained')
fig.get_layout_engine().set(h_pad=1/8)
if _MPL_36:
fig.get_layout_engine().set(h_pad=1/8)
Copy link
Member Author

@rcomer rcomer Oct 4, 2023

Choose a reason for hiding this comment

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

The deprecation note on set_constrained_layout_pads suggests that the above should work from v3.6, but I have not actually tested.
https://matplotlib.org/stable/api/figure_api.html#matplotlib.figure.Figure.set_constrained_layout_pads



@pytest.mark.natural_earth
@pytest.mark.mpl_image_compare(
filename="igh_land.png", tolerance=0.5 if _MPL_35 else 3.6)
filename="igh_land.png", tolerance=0.5 if _MPL_36 else 3.6)
Copy link
Member Author

Choose a reason for hiding this comment

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

CI fails here if the tolerance is set to 0.5 with mpl 3.5.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am confused by this because it looks like the tolerances were set at #2058, when mpl 3.5.2 would have been the newest version 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't read too much into that. Maybe the image was made with a main checkout at the time or something? Our tests are often brittle to these things, so this seems fine to me to bump one version chalking it up to we didn't know exactly which minimum version was needed.

Maybe we should think about testing with all versions of Matplotlib that we support though rather than only the newest and oldest.

@dopplershift dopplershift merged commit 515991d into SciTools:main Oct 5, 2023
2 of 14 checks passed
@QuLogic QuLogic added this to the Next Release milestone Oct 5, 2023
@rcomer rcomer deleted the mpl3.5-min-pin branch October 6, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants