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

MNT: Drop Python 3.4 from test matrix #926

Merged
merged 11 commits into from
Nov 27, 2017
Merged

Conversation

dopplershift
Copy link
Contributor

Conda-forge packages for 3.4 aren't updated any more. Move 3.5 to where
3.4 was used, use 3.6 for "latest everything".

Conda-forge packages for 3.4 aren't updated any more. Move 3.5 to where
3.4 was used, use 3.6 for "latest everything".
@dopplershift
Copy link
Contributor Author

So replacing 3.4 in the build matrix with 3.5 and conda packages means:

  • NumPy 1.9
  • Matplotlib 1.4.3
  • SciPy 0.16.0

Is it ok just to update these?

@dopplershift
Copy link
Contributor Author

And no clue why running on 3.6 breaks 2 tests....

@QuLogic
Copy link
Member

QuLogic commented Aug 16, 2017

I personally don't think we should bother with 1.3.1, but I assume this is something that the Met Office has previously had need for.

@ajdawson
Copy link
Member

1.3.1 seems a little bit too conservative, 1.4.3 is 2 major versions behind the latest and that should satisfy most needs.

These are the latest available for Python 3.5 from conda-forge.
@QuLogic
Copy link
Member

QuLogic commented Sep 17, 2017

Any idea what's up with the libgfortran install?

@dopplershift
Copy link
Contributor Author

Need to install libgfortran 1.0 package--which unfortunately isn't an explicit dependence of scipy 0.16 in defaults.

@dopplershift
Copy link
Contributor Author

Well, hey! That last commit at least got Python 3.5 with old packages to pass...

@QuLogic
Copy link
Member

QuLogic commented Oct 19, 2017

I think we kind of skipped testing with 1.4.3...

@dopplershift
Copy link
Contributor Author

Down to 1 failure on latest everything for both 2 and 3.

Baseline:
web_tiles

Result for me locally (which also fails with same RMS):
result-web_tiles

Diff:
result-web_tiles-failed-diff

Weird.

@QuLogic
Copy link
Member

QuLogic commented Oct 19, 2017

I think these are remote images that might just need to be updated.

@dopplershift
Copy link
Contributor Author

I was just looking at that. We could also bump the tolerance for Matplotlib 2.x from 2 to 2.9.

@dopplershift
Copy link
Contributor Author

Holy crap! A green build! Somebody merge it before it breaks again! 😈 🎉

if MPL_VERSION >= '2':
_STREAMPLOT_IMAGE = 'streamplot'
elif MPL_VERSION >= '1.5':
elif MPL_VERSION >= '1.4.3':
_STREAMPLOT_IMAGE = 'streamplot_1.5'
Copy link
Member

Choose a reason for hiding this comment

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

Maybe should rename these two?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done here and in the other file named for 1.5 below.

.travis.yml Outdated
else
wget http://repo.continuum.io/miniconda/Miniconda3-3.7.0-Linux-x86_64.sh -O miniconda.sh;
wget http://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh -O miniconda.sh;
Copy link
Member

Choose a reason for hiding this comment

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

Can you make these https too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

self._assert_bounds(polygon.bounds, 170, 0, 180, 10)
polygon = multi_polygon[1]
self._assert_bounds(polygon.bounds, -180, 0, -170, 10)
poly1, poly2 = multi_polygon
Copy link
Member

Choose a reason for hiding this comment

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

This was sooo annoying.

There's no point in pinning to such an old version of miniconda which
uses Python 3.4
Required for at least the old scipy.
Looks like some images were incorrectly matched to matplotlib >=1.5 when
really it was 1.4.3 that was the version when things changed. Also
update a threshold for a minor difference.
Matplotlib 2.1 now warns about axes() reusing existing instances, so
take steps to prevent that.
I'm pretty sure 7 decimal places is sufficient.
These were relying on the order of polygons, which doesn't seem to be
guaranteed by the wrapping process. It was essentially relying upon the
order of popitem() from a dict, which changed with Python 3.6. This
changes to try to identify the appropriate polygons before embarking on
testing them.
For now, let's just get the test suite back to green on Travis.
Looks like some of the tiles changed a bit, so bump the tolerance up a
bit more so that the test passes.
@djkirkham
Copy link
Contributor

djkirkham commented Nov 15, 2017

I'm getting failures when I run the tests locally: three related to ShapeReader, and a couple of image test failure when running with the latest available Matplotlib (2.1.0). I'll restart the Travis tests here.

Edit: Never mind about the Matplotlib failure, I see that it's been pinned on purpose.

@djkirkham
Copy link
Contributor

See https://github.com/djkirkham/cartopy/tree/test_failures for fixes to the ShapeReader tests. Some of them are happening because the records are being received in a different order now and the tests were indexing a list. Another one is happening because the data has apparently been extended and changed slightly (river data).

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.

6 participants