Skip to content

Use APT for libgeos on Travis #245

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

Merged
merged 6 commits into from
Dec 21, 2015
Merged

Use APT for libgeos on Travis #245

merged 6 commits into from
Dec 21, 2015

Conversation

micahcochran
Copy link
Contributor

This is for issue #235 . Using the APT binary versions of libgeos speed up Travis test times by eliminating some of the compiling. Also, dashes were added to install steps to show the install timea for dependent packages.

In testing, the overall Travis test time is about the same. It adds two additional tests builds.

The way this is setup APT installs these period. setup.py does not have a way to force local libgeos install, which is probably not a bad thing.

Micah Cochran and others added 2 commits December 17, 2015 09:59
Using the APT binary versions of libgeos speed up Travis test times by eliminating some of the compiling.  Also, dashes were added to install steps in order to show the time it takes to install dependent packages.

In testing, the overall build time is about the same.
- python: 2.7
env: BUILD_INTERNAL_GEOS=true
- python: 3.5
env: BUILD_INTERNAL_GEOS=true
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I would have just picked two of the existing test runs and set them to use the internal geos. It'll reduce the amount of work that Travis has to do.

@WeatherGod
Copy link
Member

So, regardless of whether or not the libgeos was built locally or not, these tests will always use the system libgeos? Not sure if that was what you meant to say.

@micahcochran
Copy link
Contributor Author

@WeatherGod disregard that.

Through trial and error, I did figure out how to apt install libgeos on some instances and not on others. It made the configuration cumbersome and it is not reflected in this PR.

I like the idea of having different versions of requirements to test against: Current, last release, and minimal requirements. I think that should be another pull request to the travis,yaml. The issue here is travis is kinda slow with the current configuration.

Do you want me to use perhaps Python 3.3 to test the compiling of the internal libGEOS?
Otherwise, I'm ready for a merge.

@micahcochran
Copy link
Contributor Author

I almost forgot. I removed virtualenv because Travis already uses virtualenv by default.

@jenshnielsen
Copy link
Member

The Travis virtualenv comes with there version of numpy so not using our own virtual env means that we pick that one that Travis has unless we pass a specific version or pass the --upgrade flag

@micahcochran
Copy link
Contributor Author

With the current configuration for Python 2.6, pip uninstalls numpy 1.9.1 and then installs 1.6.2.

The current virtualenv version (currently in the repository) installs numpy 1.10.2. Is that preferable?

I think specifying env: NUMPY=numpy>=1.10,<1.11 on one of the versions will accomplish testing the latest & greatest.

What do you want to see before merging?

@WeatherGod
Copy link
Member

Let's have py27 and py35 be the "latest and greatest" environments, while py26 and py33 be the "minimum requirements" environments. In addition, the minimum requirements environments should use our build of libgeos. Any other environments should use the default provided by travis.

Downloads and compiles external version of libgeos from github.  Installs version that are "latest and greatest", "middle of the road", "minimum requirements", and "current development" environment.

The "middle of the road" Python 3.4.  Gets everything done in less than 3 minutes.

The "current development" environment is allowed to fail.  It is more of a diagnostic to address future possible incompatibilities.

For the "minimum requirements" environment, this took a bit of testing.  The earliest versions that I could get to work on Python 2.6 and 3.3 were matplotlib 1.2.0 and numpy 1.7.0.  Python 2.6 did work with earlier versions of numpy.  If desired, a travis test matrix could be built to test out all the possibilities to truly find the minimum.  See config for details.

Total build time is similar to before this, but this tests the boundaries a bit more.

There is more work that could be done, but I think this is a completely better system from the current.
@WeatherGod
Copy link
Member

... and it looks like Travis had some sort of change and can't parse the yaml file correctly...

@micahcochran
Copy link
Contributor Author

@WeatherGod My fault in copying and pasting... I also forgot to make "3.5-dev" to allow failure. I will do that in the next commit.

@micahcochran
Copy link
Contributor Author

The development version fails here with the unit test:

ERROR: test_no_cyc2 (main.TestShiftGrid)

Traceback (most recent call last):
File "lib/mpl_toolkits/basemap/test.py", line 105, in test_no_cyc2
grid, lon = shiftgrid(lonin[len(lonin)/2], gridin, lonin, start=False)
IndexError: only integers, slices (:), ellipsis (...), numpy.newaxis (None) and integer or boolean arrays are valid indices

This is the same DeprecationWarning, but it is an IndexError in the development version. The math here lonin[len(lonin)/2] needs to use a double slash (//). Why? Well, that is the floor division operator. If both numbers are integers, the resulting number will also be an integer.

Example from Python 3.4.3

In [1]: 5/3
Out[1]: 1.6666666666666667

In [2]: 5//3
Out[2]: 1

In [3]: 5.0//3.0
Out[3]: 1.0

Should this IndexError/DeprecationWarning be address in this PR or another?

I assume there is an option enabled to make warning into errors in Python development version in order to keep warnings out of the mainline code.

EDIT: The cause of the DeprecationWarning/IndexError is caused by using a floating point number as the index of a list.

@micahcochran
Copy link
Contributor Author

Notes about the current yaml configuration:

Downloads and compiles external version of libgeos from github. Installs version that are "latest and greatest", "middle of the road", "minimum requirements", and "current development" environment.

The "middle of the road" Python 3.4. Gets everything done in less than 3 minutes.

The "current development" environment is allowed to fail. It is more of a diagnostic to address future possible incompatibilities.

For the "minimum requirements" environment, this took a bit of testing. The earliest versions that I could get to work on Python 2.6 and 3.3 were matplotlib 1.2.0 and numpy 1.7.0. Python 2.6 did work with earlier versions of numpy. If desired, a travis test matrix could be built to test out all the possibilities to truly find the minimum. See config for details.

Total build time is similar to before this, but this tests the boundaries a bit more.

There is more work that could be done, but I think this is a completely better system from the current.

env:
- NUMPY=numpy>=1.10.0
- MPL=matplotlib>=1.5.0
- BUILD_LIBGEOS=3.5.0
Copy link
Member

Choose a reason for hiding this comment

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

Rather than hardcoding the latest version which will for sure fall out off sync I would suggest these envs to pass --upgrade to pip that way we are sure that the latest version is installed.

@micahcochran
Copy link
Contributor Author

@jenshnielsen I have incorporated your suggestions in the most recent commit. The least amount of specifying a particular version that can be done, makes the configuration less like to be updated. Python "nightly" is better.

@WeatherGod
Copy link
Member

This is looking real good now. We got one travis instance that is slow right now, but that is just luck of the draw, sometimes.

@WeatherGod
Copy link
Member

Ok, let's merge this and any additional improvements can be made in new PRs.

WeatherGod added a commit that referenced this pull request Dec 21, 2015
@WeatherGod WeatherGod merged commit 25fcba6 into matplotlib:master Dec 21, 2015
@WeatherGod
Copy link
Member

Great work!

@micahcochran
Copy link
Contributor Author

Thanks @WeatherGod

@micahcochran micahcochran deleted the travis-apt-try2 branch December 21, 2015 18:13
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.

3 participants