Travis #1397

Merged
merged 11 commits into from Nov 23, 2012

Projects

None yet

8 participants

@syhw
Contributor
syhw commented Nov 23, 2012

Travis is a continuous integration service that builds the project and check if tests pass. https://travis-ci.org/

This pull request contains:

  • the travis config (.travis.yml) file that tells what to install of the VM (before_install:); how to install (install:) the (python) requirements.txt and sklearn; and how to make tests (script:)
  • the pip-formated requirements of sklearn (requirements.txt)
  • the link to the build status image (README.rst), of course you need to change this link to https://secure.travis-ci.org/scikit-learn/scikit-learn.png once you will have merged.

To setup travis, you need one more thing than pulling this, as described here http://about.travis-ci.org/docs/user/getting-started/#Step-one%3A-Sign-in =>
go there https://travis-ci.org/ and sign-in with the scikit-learn github account (OAuth), give read and write permissions (for travis hooks on push), go to your profile https://travis-ci.org/profile and activate scikit-learn. Now you should have a continuously updated build status and a glimpse of it in the readme! :)

The full report is there https://travis-ci.org/SnippyHolloW/scikit-learn and so, once merged, there https://travis-ci.org/scikit-learn/scikit-learn

Cheers,
Keep up the good scikit-learn coming! :)

@amueller
Member

Thanks, this is much appreciated! 👍
Good travis integration would be really, really great!

cc @erg

@mblondel
Member

Excellent. Seems like it's already working for this PR.

@ogrisel
Member
ogrisel commented Nov 23, 2012

Maybe there is a way to pass virtualenv --system-site-packages?

@syhw
Contributor
syhw commented Nov 23, 2012

It should be the default.

@syhw
Contributor
syhw commented Nov 23, 2012

Apparently, the virtualenv of Travis cannot use apt-get packages http://about.travis-ci.org/docs/user/languages/python/#Travis-CI-Uses-Isolated-virtualenvs I rolled back to installing scipy via pip.

@jaquesgrobler
Member

This is very nice! I was actually gonna read up on this and attempt this myself! haha.. I'll put in some time to go through your work.. Thanks a lot and very cool 👍

@ogrisel
Member
ogrisel commented Nov 23, 2012

Building a full scipy for each newly checked-in commit in each PR is wasting way too much CPU for nothing.

I think we should do the same hack as scikit-image:

https://github.com/scikit-image/scikit-image/blob/master/.travis.yml

@GaelVaroquaux
Member

I think we should do the same hack as scikit-image:

https://github.com/scikit-image/scikit-image/blob/master/.travis.yml

Great suggestion. I am +1!

@dnouri
Contributor
dnouri commented Nov 23, 2012

FWIW, you can also create a new virtualenv, one with --system-site-packages in Travis without problems:

before_install:
 - deactivate
 - virtualenv --system-site-packages ~/virtualenv/this
 - source ~/virtualenv/this/bin/activate

This way it'll have access to all packages installed via apt-get.

@GaelVaroquaux
Member

before_install:

  • deactivate
  • virtualenv --system-site-packages ~/virtualenv/this
  • source ~/virtualenv/this/bin/activate

This way it'll have access to all packages installed via apt-get.

If this works, its even better.

@syhw
Contributor
syhw commented Nov 23, 2012

Thanks @dnouri ! It works 👍
Build+test time is now ~4min instead of ~10-13min. Reinstalling a full python stack (with the "erlang trick") should be longer I guess.

@GaelVaroquaux
Member

Build+test time is now ~4min instead of ~10-13min.

Excellent. This is really cool. I am +1 for merge.

@mblondel
Member

We should tell scikit-image people.

@ogrisel ogrisel commented on an outdated diff Nov 23, 2012
@@ -1,4 +1,5 @@
.. -*- mode: rst -*-
+.. image:: https://secure.travis-ci.org/SnippyHolloW/scikit-learn.png
@ogrisel
ogrisel Nov 23, 2012 Member

This will need to be updated to:

https://secure.travis-ci.org/scikit-learn/scikit-learn.png

at least I guess.

@amueller
Member

Looks great, +1 for merge.
Do we know which architecture / library versions are used? Can we have multiple ones?

@ogrisel ogrisel commented on an outdated diff Nov 23, 2012
@@ -0,0 +1,13 @@
+language: python
+python:
+ - "2.7"
+before_install:
+ - deactivate
+ - sudo apt-get update -qq
+ - sudo apt-get install -qq python-scipy python-nose
+ - virtualenv --system-site-packages ~/virtualenv/this
+ - source ~/virtualenv/this/bin/activate
+install:
+ - pip install -q -r requirements.txt --use-mirrors
@ogrisel
ogrisel Nov 23, 2012 Member

We no longer need the requirements as they are installed using apt, do we?

@ogrisel
Member
ogrisel commented Nov 23, 2012

Indeed it would be great to have 32 bit arch.

@syhw
Contributor
syhw commented Nov 23, 2012

Yes @ogrisel I put that the link to the travis build should be changed after the pull in the pull-request. Should I change it now before the pull? (it would break the image.) Also, yes we can remove the requirements, should I remove requirements.txt too? I think it's a good (because it's standard) to enforce them for others to install sklearn the pip way.

@amueller Travis-CI uses a 32bits Ubuntu 12.04 (server), the packages it uses are here http://packages.ubuntu.com/precise/

@ogrisel
Member
ogrisel commented Nov 23, 2012

Yes please change it now.

@GaelVaroquaux
Member

Do we know which architecture / library versions are used? Can we have multiple
ones?

We can, but it will induce addition compution load that we give slower
tracking of the pull requests. I'd rather rely on jenkins for this.

@ogrisel
Member
ogrisel commented Nov 23, 2012

I think we are good to go for merging, thanks @SnippyHolloW .

@ogrisel
Member
ogrisel commented Nov 23, 2012

Any other comments before merging (I am +1)?

@mblondel
Member

+1 for merge

@ogrisel ogrisel merged commit e256540 into scikit-learn:master Nov 23, 2012
@ogrisel
Member
ogrisel commented Nov 23, 2012

Merged, thanks again @SnippyHolloW

@mblondel
Member

Someone needs to configure Travis as @SnippyHolloW explained in the beginning but I don't know how to login with the scikit-learn organization.

@mblondel
Member

I think I managed to activate it.

https://travis-ci.org/scikit-learn/scikit-learn

@ogrisel
Member
ogrisel commented Nov 23, 2012

Thanks, I was on the same page and was confused to see it turning on magically :)

@mblondel
Member

I wonder if we can have Travis check a few recent PRs (hopefully not all 58 of them ;).

@ogrisel
Member
ogrisel commented Nov 23, 2012

All you need to do is merge master to them.

@amueller
Member

Woot! thanks @SnippyHolloW !

@satra
Member
satra commented Nov 23, 2012

thanks @SnippyHolloW - we were using the erlang trick for a different project - this makes it much nicer!

others: if you turn it on your own sklearn fork (via the Admin button), it will check a PR the moment you push a change to your branch corresponding to the PR.

@satra
Member
satra commented Nov 23, 2012

you can also do it through: https://travis-ci.org/profile

@mblondel
Member

Apparently the button https://secure.travis-ci.org/scikit-learn/scikit-learn.png indicates the status of the latest pull request... (currently failing)

@dnouri
Contributor
dnouri commented Nov 24, 2012

Turns out there's an easier way of using a virtualenv with --system-site-packages, without the deactivate-create-source dance. Needs just this in .travis.yml:

virtualenv:
  system_site_packages: true

I've written a little blog post about this: http://danielnouri.org/notes/2012/11/23/use-apt-get-to-install-python-dependencies-for-travis-ci/

@mblondel
Member

@dnouri: Can you send us a PR? Thanks a ton!

@ogrisel
Member
ogrisel commented Nov 26, 2012

@dnouri nice. Would you like to submit a PR?

@ogrisel
Member
ogrisel commented Nov 26, 2012

@mblondel just a few seconds away 2 days after the last comment :)

@syhw
Contributor
syhw commented Nov 26, 2012

I did what @dnouri last told
syhw@343f895
it does not shorten the build on Travis. I can PR later today if you
want.

@mblondel
Member

@mblondel just a few seconds away 2 days after the last comment :)

I laughed out loud in the lab :)

@ogrisel
Member
ogrisel commented Nov 26, 2012

@SnippyHolloW it's not faster but it's simpler / cleaner so +1 for a PR.

@syhw
Contributor
syhw commented Nov 26, 2012

On Mon, Nov 26, 2012 at 2:47 PM, Olivier Grisel notifications@github.comwrote:

@SnippyHolloW https://github.com/SnippyHolloW it's not faster but it's
simpler / cleaner so +1 for a PR.

Ack, I merge scikit-learn's master into mine and I do that PR tonight when
I get back home.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment