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

Update image tests for matplotlib 2.1. #1070

Merged
merged 7 commits into from
Jul 12, 2018
Merged

Conversation

pelson
Copy link
Member

@pelson pelson commented May 17, 2018

Looks like a few tests were failing due to font differences, and others due to changes in underlying image sources.

@dopplershift
Copy link
Contributor

Unfortunately, there are still 21 failing tests. 😭

@bjlittle
Copy link
Member

bjlittle commented May 19, 2018

I think we need to introduce some perceptual image hashing to ease the pain of graphical testing in cartopy.

I'm happy to make the approach we use in iris a generic first class citizen, so that we could easily add it as a dependency and use it for graphical testing. I think that it might make life slightly more bearable.

I know of other scitools packages that would also benefit, and perhaps there may be wider community appetite.

Thoughts @pelson...

@pelson
Copy link
Member Author

pelson commented May 21, 2018

Yep, agreed. I considered doing the work myself instead of this PR. I don't really want to introduce a change to the tests that means we could not test with older versions of matplotlib - I'd like the thing to be additive, not substitutive.

Pulling out the machinery from Iris into its own project is the obvious next step, and I think that is something we should focus on next.

@QuLogic
Copy link
Member

QuLogic commented May 25, 2018

I'm not sure you want to update the barb/streamplot results before #1042, though I'm not sure which one has the greater effect. I don't see any of these ones failing on that PR with 2.1.1.

@pelson
Copy link
Member Author

pelson commented Jul 10, 2018

OK, I'm going to take this one on the chin and merge #1042 even though the tests are failing. The reason I'm doing this is that:

a) master's tests have been failing for way too long
b) I want to get masters test's working again, and there are tendrils to doing that in this PR

@pelson
Copy link
Member Author

pelson commented Jul 10, 2018

So I totally sent that last message to the wrong PR 🤣. I'm working on this PR, and decided to merge #1042 so that I can move this forwards.

@pelson pelson force-pushed the fix_tests branch 2 times, most recently from 61111ae to d5b6296 Compare July 11, 2018 11:55
@pelson
Copy link
Member Author

pelson commented Jul 12, 2018

🎉 - this brings the tests back online. I'm keen to address the testing issue once and for all in terms of truly reproducible image tests and data sources, but I'm reasonably content to address that in slower time. For now, we need to get the tests back up and passing.

cc @dopplershift if you'd care to do the honours before the weekend's sprints 😉 ?

@dopplershift
Copy link
Contributor

🎉

@dopplershift dopplershift merged commit 024b0c4 into SciTools:master Jul 12, 2018
@pelson pelson deleted the fix_tests branch July 12, 2018 16:15
@dopplershift dopplershift added this to the 0.17 milestone Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants