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

Upgrade tests to use matplotlib v1.4/v1.5 #354

Closed
pelson opened this issue Sep 20, 2013 · 19 comments
Closed

Upgrade tests to use matplotlib v1.4/v1.5 #354

pelson opened this issue Sep 20, 2013 · 19 comments

Comments

@pelson
Copy link
Member

pelson commented Sep 20, 2013

Deprecate v1.2.0 support after v0.10.

@pelson pelson removed this from the 0.12 milestone Jun 18, 2014
@QuLogic
Copy link
Member

QuLogic commented Sep 4, 2015

Travis seems to be using 1.3.1, so I guess this is fixed?

@pelson
Copy link
Member Author

pelson commented Sep 4, 2015

Seems reasonable. Maybe we should re-use the issue to upgrade to v1.4 (or v1.5 when it arrives)

@QuLogic
Copy link
Member

QuLogic commented Oct 25, 2015

@pelson Do you know if figure autoscaling was changed in matplotlib 1.4? It's the only thing that's causing a really big difference in results. For example, test_view_lim_autoscaling produces:

AssertionError: 
Arrays are not almost equal to 6 decimals

(mismatch 100.0%)
 x: array([[ 86.125047,  52.516846],
       [ 86.69595 ,  52.862473]])
 y: array([[ 86.124337,  52.515705],
       [ 86.696966,  52.863721]])

@QuLogic QuLogic changed the title Upgrade tests to use matplotlib v1.3 Upgrade tests to use matplotlib v1.4/v1.5 Oct 25, 2015
@QuLogic
Copy link
Member

QuLogic commented Oct 26, 2015

These images require a small change to the baseline due to minor differences in the corners of the axes lines (~0.1-0.3 RMS):

  • lib/cartopy/tests/mpl/baseline_images/mpl/test_img_transform/regrid_image.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_mpl_integration/barbs_plate_carree.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_mpl_integration/global_scatter_wrap.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_mpl_integration/multiple_projections1.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_ticks/xticks_cylindrical.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_ticks/xticks_no_transform.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_ticks/xyticks.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_ticks/yticks_cylindrical.png
  • lib/cartopy/tests/mpl/baseline_images/mpl/test_ticks/yticks_no_transform.png

These tests show larger issues:

@QuLogic
Copy link
Member

QuLogic commented Oct 26, 2015

I think many of these are intended changes and/or bugfixes, so I'm inclined to simply replace the baseline images. The first streamplot change might be a bug, but it's been so long, I don't know if it should be changed now. matplotlib/matplotlib@b8726d0 is a bit strange, but the test image was specifically changed, so I guess it's intended. I'm not totally sure about matplotlib/matplotlib@f511ef9 though.

@QuLogic
Copy link
Member

QuLogic commented Oct 26, 2015

There's one more change causing failures with 1.4.3:

  • cartopy.tests.mpl.test_mpl_integration.test_pcolormesh_goode_wrap (RMS: 32.5231528534), cartopy.tests.mpl.test_mpl_integration.test_pcolormesh_mercator_wrap (RMS: 26.5532499842). - caused by matplotlib/matplotlib@44ff97e

I think this one might be a bug. The left edge is cropped a little bit, and it's pretty evident from the Interrupted Goode Homolosine image.
result-pcolormesh_goode_wrap

@QuLogic
Copy link
Member

QuLogic commented Oct 26, 2015

With 1.5.x, there are some more images with small changes (RMS < 1):

  • cartopy.tests.mpl.test_features.test_gshhs
  • cartopy.tests.mpl.test_images.test_imshow
  • cartopy.tests.mpl.test_images.test_imshow_projected
  • cartopy.tests.mpl.test_images.test_pil_Image
  • cartopy.tests.mpl.test_images.test_stock_img
  • cartopy.tests.mpl.test_mpl_integration.test_barbs
  • cartopy.tests.mpl.test_mpl_integration.test_global_contourf_wrap_new_transform
  • cartopy.tests.mpl.test_mpl_integration.test_global_contourf_wrap_no_transform
  • cartopy.tests.mpl.test_mpl_integration.test_pcolormesh_goode_wrap
  • cartopy.tests.mpl.test_mpl_integration.test_streamplot
  • cartopy.tests.mpl.test_web_services.test_wms
  • cartopy.tests.mpl.test_web_services.test_wmts

All are caused by matplotlib/matplotlib@2a17839 (upgrade of Agg) and since the changes are nearly invisible, I think simply replacing them is the way to go.

There are a few more larger changes to bisect...

@QuLogic
Copy link
Member

QuLogic commented Oct 26, 2015

It turns out almost all the images need to regenerated due to the above mentioned upgrade of Agg in the upcoming matplotlib 1.5.0. There are a couple other major differences:

  • cartopy.tests.mpl.test_examples.test_global_map (RMS: 0.777689776768), cartopy.tests.mpl.test_mpl_integration.test_global_map (RMS: 0.585012464255), cartopy.tests.mpl.test_mpl_integration.test_multiple_projections (RMS: 1.87054537502) - caused by matplotlib/matplotlib@37927b2
  • cartopy.tests.mpl.test_gridliner.test_grid_labels (RMS: 25.706532414), cartopy.tests.mpl.test_img_transform.test_regrid_image (RMS: 4.26279634356), cartopy.tests.mpl.test_shapely_to_mpl.test_contour_interiors (RMS: 15.1097057659), cartopy.tests.mpl.test_ticks.test_set_xticks_cylindrical (RMS: 4.71076095233), cartopy.tests.mpl.test_ticks.test_set_xyticks (RMS: 6.46914144226), cartopy.tests.mpl.test_ticks.test_set_yticks_cylindrical (RMS: 8.16085320294), cartopy.tests.mpl.test_ticks.test_set_yticks_no_transform (RMS: 8.41449505813) - caused by matplotlib/matplotlib@c6d0183

I think both of the above are intended changes.

@QuLogic
Copy link
Member

QuLogic commented Oct 26, 2015

You can see the resulting images in this branch. There is still something weird going on because cartopy.tests.mpl.test_images.test_imshow and cartopy.tests.mpl.test_images.test_pil_Image both fail even though they byte-for-byte produce the same image as expected.

@QuLogic
Copy link
Member

QuLogic commented Oct 26, 2015

@pelson Let me know if we should file any bugs in matplotlib; not sure we'll be able to get them into 1.5.0, at this point.

@pelson
Copy link
Member Author

pelson commented Oct 30, 2015

This is awesome work @QuLogic - super thorough, and really useful, thank you!

Let me know if we should file any bugs in matplotlib

Yes we should. Whether they are fixed in 1.5 or a subsequent release is not as important as ensuring we report them.

Next steps: I think we should consider our image testing strategy within cartopy - the many minor differences in agg rendering show that we are way to sensitive to minor rendering changes, yet some of the other tests show that we are way too insensitive to some other key changes. Obviously we are not going to solve that problem here and now, but it is worth mentioning and starting to think about.

As for next steps, I think it would be healthy to be able to run the cartopy's matplotlib tests against multiple matplotlib versions for a few releases before we drop support for v1.3. Doing so means we need to define a results structure which allows us to have multiple image result directories (based on the matplotlib version). Would you mind taking a look at that?

The next concern is the fact that the cartopy repository is only getting bigger due to these image changed. Already I find it frustrating to do a clean checkout of the cartopy repo due to the time to download, for some reason my git seems to throttle after a number of Mb. One option then would be to pull all of the image tests out of the core repository, and to put them into their own repo which we link to in here (perhaps using git submodules). What are your thoughts on doing that?

@pelson
Copy link
Member Author

pelson commented Jan 6, 2016

matplotlib/matplotlib@44ff97e (matplotlib/matplotlib#5370.)

I've been digging on this, and while the result is different, I think the problem is related to the new need to do a relim and autoscale_view as per #647 (comment). Some investigation of why this has come about is needed - it may be that the draw pipeline has changed subtly, and we need to account for that in the draw call on GeoAxes.

pelson added a commit to pelson/cartopy that referenced this issue Jan 6, 2016
pelson added a commit to pelson/cartopy that referenced this issue Jan 6, 2016
pelson added a commit to pelson/cartopy that referenced this issue Jan 6, 2016
pelson added a commit to pelson/cartopy that referenced this issue Jan 7, 2016
@QuLogic
Copy link
Member

QuLogic commented Jan 10, 2016

So what's the minimum matplotlib version to support?

@pelson
Copy link
Member Author

pelson commented Jan 13, 2016

So what's the minimum matplotlib version to support?

Somewhat arbitrarily v1.3. One of my own motivations for that is my employers stack is still on that version currently.

@pelson pelson added this to the 0.14 milestone Jan 28, 2016
pelson added a commit that referenced this issue Jan 28, 2016
Patched QuadMesh to compute the datalimits of the boundary, not the bounding box (see #354).
@QuLogic
Copy link
Member

QuLogic commented Feb 2, 2016

cartopy.tests.mpl.test_images.test_imshow and cartopy.tests.mpl.test_images.test_pil_Image both fail even though they byte-for-byte produce the same image as expected.

OK, I figured out this one; they both produce a different image from the stored one, but the other test that uses the same result overwrites it with the correct one. A bit of a peril of re-using the same result image name.

@QuLogic
Copy link
Member

QuLogic commented Feb 2, 2016

I think maybe mpl/baseline_images/mpl/test_mpl_integration/pcolormesh_limited_area_wrap.png got broken somehow.

@QuLogic
Copy link
Member

QuLogic commented Feb 2, 2016

Also, the new-ish mpl/baseline_images/mpl/test_shapely_to_mpl/poly_interiors.png is slightly different, maybe due to different snapping.

@QuLogic
Copy link
Member

QuLogic commented Mar 2, 2016

Should be done now.

@QuLogic QuLogic closed this as completed Mar 2, 2016
@QuLogic QuLogic modified the milestones: 0.14, 0.14.0 Mar 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants