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

unpin mpl #3468

Merged
merged 4 commits into from
Oct 17, 2019
Merged

unpin mpl #3468

merged 4 commits into from
Oct 17, 2019

Conversation

bjlittle
Copy link
Member

@bjlittle bjlittle commented Oct 16, 2019

This PR unpins matplotlib to use the latest version 3.1.1, at this point in time.

Note that, in general the image differences were due to:

  • legend label positioning
  • axis ticks and label positioning
  • default levels for contours (the cause for the majority of failures)
  • axes get_position returning a different bottom

Users that explicitly specify the contour levels will not (in general) see a (subtle) change in the default colours of the rendered contour lines chosen by matplotlib 3.x.

The graphical tests were run against the following versions of matpotlib - 3.0.1, 3.0.2, 3.0.3, 3.1.0 and 3.1.1. Note that, 3.0.0 suffers from a bug that causes a failure to import matplotlib when used with backend-fallback on Python 3.7.x - this is fixed by 3.0.1.


Closes #3400

@bjlittle bjlittle added this to the v3.0.0 milestone Oct 16, 2019
@bjlittle bjlittle added this to In Progress in Iris v3.0.0 via automation Oct 16, 2019
@@ -8,7 +8,7 @@ cartopy
cf-units>=2
cftime
dask[array]>=2 #conda: dask>=2
matplotlib>=2,<3
matplotlib
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that, we still want to enforce proj4<6... for now.

@@ -131,8 +131,7 @@ def main():
width = left - first_plot_left + width

# Add axes to the figure, to place the colour bar
colorbar_axes = fig.add_axes([first_plot_left, bottom + 0.07,
width, 0.03])
colorbar_axes = fig.add_axes([first_plot_left, 0.18, width, 0.03])
Copy link
Member Author

@bjlittle bjlittle Oct 16, 2019

Choose a reason for hiding this comment

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

The bottom returned from get_position on line +127 now aligns with the actual bottom of the plot axes - this wasn't the case for matplotlib<3.

The fix here is to explicitly set the bottom of the colorbar_axes to align with this example from matplotlib=2.*

@bjlittle
Copy link
Member Author

bjlittle commented Oct 16, 2019

❗ Requires the associated SciTools/test-iris-imagehash#26 PR to be merged first.

@bjlittle bjlittle moved this from In Progress to Waiting for Review in Iris v3.0.0 Oct 16, 2019
@bjlittle
Copy link
Member Author

@lbdreyer @pp-mo @stephenworsley Any takers for this PR and the associated SciTools/test-iris-imagehash#26 ?

@lbdreyer lbdreyer self-assigned this Oct 16, 2019
@lbdreyer lbdreyer moved this from Waiting for Review to In Progress in Iris v3.0.0 Oct 16, 2019
@bjlittle
Copy link
Member Author

bjlittle commented Oct 16, 2019

@lbdreyer I'm happy to add the diff images to this PR if it helps clarify?

@lbdreyer
Copy link
Member

@lbdreyer I'm happy to add the diff images to this PR if it helps?

Don't worry! I'm happy looking at them locally

@@ -115,11 +115,11 @@ script:
- >
if [[ ${TEST_TARGET} == 'default' ]]; then
export IRIS_REPO_DIR=${INSTALL_DIR};
python -m iris.tests.runner --default-tests --system-tests --print-failed-images;
Copy link
Member Author

@bjlittle bjlittle Oct 16, 2019

Choose a reason for hiding this comment

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

Retire this testing infrastructure - it's flaky and pre-dates the existing workflow using perceptual image hashing.

If there are graphical failures on travis-ci then the developer will rerun locally, which is an acceptable workflow now that we have the necessary idiff capability and repeatable travis-ci environments locally. So this --print-failed-images capability is really no longer needed... so let's take the opportunity to purge old technical debt 😜

@bjlittle
Copy link
Member Author

Requires SciTools/test-iris-imagehash#27 to be merged first.

@bjlittle
Copy link
Member Author

Extra test failures were due to the change in mpl3 contour levels picking slighly different colours.

That said, the example_tests.test_atlantic_profiles.TestAtlanticProfiles.test_atlantic_profiles.1.png failed due to mpl3 having slighly different x and y ticks positions, which is acceptable.

@bjlittle
Copy link
Member Author

bjlittle commented Oct 16, 2019

This is curious.... but okay, mpl3 has moved the ticks on x and y...

result-example_tests test_atlantic_profiles TestAtlanticProfiles test_atlantic_profiles 1-failed-diff

@lbdreyer
Copy link
Member

result-example_tests test_atlantic_profiles TestAtlanticProfiles test_atlantic_profiles 1-failed-diff

This one kinda concerns me. I find it strange that there are colour differences in the plot but not the colour bar 😕

@lbdreyer
Copy link
Member

This one kinda concerns me. I find it strange that there are colour differences in the plot but not the colour bar 😕

Okay I think the colour differences are due to the scatter points (and the rest of the plot) shifting slightly, rather than the values of the scatter points actually being different!

@bjlittle
Copy link
Member Author

Yeah, I agree... it might just be artifacts of the overplotting. Difficult to really say...

@bjlittle
Copy link
Member Author

bjlittle commented Oct 16, 2019

Just leaving a bread-crumb trail for my future self (or anyone else) to repeat the testing...

#!/usr/bin/env bash


# steps...
#
# conda create -n iris-mpl -c conda-forge python=3.7 cartopy cf-units cftime dask matplotlib netcdf4 numpy pyke scipy six nose imagehash pip filelock iris-sample-data nc-time-axis
# . activate iris-mpl
# conda install -c conda-forge python-eccodes
# conda install -c conda-forge --no-deps iris-grib
# pip install -e .  # iris dev branch
#
# then run this script...
# python lib/iris/test/idiff.py --resultdir iris_image_test_output

BASE=lib/iris/tests

python -m iris.tests.runner --example-tests
python ${BASE}/experimental/test_animate.py
python ${BASE}/integration/plot/test_plot_2d_coords.py
python ${BASE}/integration/plot/test_vector_plots.py
python ${BASE}/test_mapping.py
python ${BASE}/test_plot.py
python ${BASE}/test_quickplot.py

@bjlittle
Copy link
Member Author

bjlittle commented Oct 16, 2019

@lbdreyer Tests are looking good.... all the graphical test failures have been hiding a doc building failure, which is the last thing to investigate 🕵

@@ -64,7 +64,6 @@
'sphinx.ext.imgmath',
'sphinx.ext.intersphinx',
'matplotlib.sphinxext.mathmpl',
'matplotlib.sphinxext.only_directives',
Copy link
Member Author

Choose a reason for hiding this comment

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

After a little bit of investigation, I happened across matplotlib/matplotlib#11295...

In a nutshell, ditch the matplotlib custom directives, which have been retired. The impact is simply to switch from matplotlib to the appropriate sphinx directives i.e.,

  • replace .. htmlonly:: -> .. only:: html, and
  • replace .. latexonly:: -> .. only:: latex

@bjlittle
Copy link
Member Author

@bjlittle bjlittle moved this from In Progress to Waiting for Review in Iris v3.0.0 Oct 16, 2019
@bjlittle
Copy link
Member Author

@lbdreyer Finally..... 😌 I think this is now good to go, thanks 👍

@lbdreyer lbdreyer merged commit 65a2151 into SciTools:master Oct 17, 2019
Iris v3.0.0 automation moved this from Waiting for Review to Done Oct 17, 2019
@lbdreyer
Copy link
Member

Thanks @bjlittle !

@bjlittle bjlittle deleted the unpin-mpl branch October 29, 2019 10:16
trexfeathers pushed a commit to trexfeathers/iris that referenced this pull request Jan 13, 2020
* unpin mpl

* additional tests + retire print-failed-images HTML

* fix docs

* update the friggin license headers year
trexfeathers added a commit to trexfeathers/iris that referenced this pull request Jan 13, 2020
trexfeathers added a commit to trexfeathers/iris that referenced this pull request Jan 13, 2020
…ible by dropping Travis Py2 support.

This reverts commit 04a96b7.
pp-mo pushed a commit to pp-mo/iris that referenced this pull request Jan 14, 2020
trexfeathers added a commit that referenced this pull request Jan 16, 2020
* Bump version to 2.4.0rc0.

* unpin mpl (#3468)

* Merge pull request #3301 from bayliffe/fastpercentilemethod_mask_test

Analysis percentile method - update applicability test for fast_percentile_method

* Have Travis test with iris-grib, remove problem tests (#3469)

* Have Travis test with iris-grib, remove problem tests

* mock GribInternalError correctly

* Update license headers

* account for changes in handling of grib message defaults

* Test against the latest version of python-eccodes

* Moved irir-grib skip to iris.tests

* Merge pull request #2608 from cpelley/PICKLEABLE_FORMATS

TEST: Extends #2569 to unpickle

* _regrid_area_weighted_array: Move axes creation over which weights are calculated to before loop (#3519)

* Purge iris.experimental.regrid np<1.7 support (#3539)

* Add NameConstraint with relaxed name loading (#3463)

* _regrid_area_weighted_array:  move indices variable nearer to use (#3564)

* _regrid_area_weighted_array: Tweak variable order to near other use in code (#3571)

* Fix problems with export and echo command. (#3577)

* Pushdocs fix2 (#3580)

* Revert to single-line command for doctr invocation.

* Added script comment, partly to force Github respin.

* Bracketed six.moves and __future__ imports.

* Fixes required due to the release of iris-grib v0.15.0 (#3582)

* Fix python-eccodes pin in travis (#3593)

* PI-2472: Optimise the area weighted regridding routine (#3598)

* PI-2472: Tweak area weighting regrid move xdim and ydim axes (#3594)

* _regrid_area_weighted_array: Set axis order to y_dim, x_dim last dimensions

* _regrid_area_weighted_array: Extra tests for axes ordering

* PI-2472: Tweak area weighting regrid enforce xdim ydim (#3595)

* _regrid_area_weighted_array: Set axis order to y_dim, x_dim last dimensions

* _regrid_area_weighted_array: Extra tests for axes ordering

* _regrid_area_weighted_array: Ensure x_dim and y_dim

* PI-2472: Tweak area weighting regrid move averaging out of loop (#3596)

* _regrid_area_weighted_array: Refactor weights and move averaging outside loop

* Pin pillow to make graphics tests work again. (#3630)

* PI-2472: Area weighted regridding (#3623)

* The Area-weights routine is refactored into the "__prepare" and "__perform" structure, in-line with our other regridding methods.
* The area-weights are now computed in the "__prepare", so are calculated once.
* The information required for checking the regridding weights and target grid info are now cached on the "regridder" object.
* This is inline with the general use already described in the documentation.

* pep8 conformance.

* pep8 compliance.

* Allow some 'key=None' args in Geostationary creation. (#3628)

LGTM

* Allow some 'key=None' args in Geostationary creation.

* Integration test loading netcdf geostationary without offset properties.

* pep8 conformance.

* pep8 conformance.

* pep8 conformance.

* test_NameConstraint get mock from iris.tests.

* Remove use of super() in _constraints.py for Py2 compatibility.

* Updated license headers.

* Updated iris-grib reference in extensions.txt.

* Py2 support for iris-grib in Travis.

* Updates for auto docs for Iris 2.4 release.

* What's new entry to unpinning mpl.

* Edited Py2 support for iris-grib in Travis.

* Renamed whatsnew contributions folder for v2.4.

* Hacked tests.integration.test_grib2 to avoid import error from iris-grib version < 0.15.

* Only test grib with python 3.

* Compiled v2.4 whatsnew.

Co-authored-by: Martin Yeo <[email protected]>
Co-authored-by: Bill Little <[email protected]>
Co-authored-by: stephenworsley <[email protected]>
Co-authored-by: abooton <[email protected]>
Co-authored-by: lbdreyer <[email protected]>
Co-authored-by: Emma Hogan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Iris v3.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

matplotlib 3 support
2 participants