search

nipy / PySurfer

Cortical neuroimaging visualization in Python
https://pysurfer.github.io/
BSD 3-Clause "New" or "Revised" License
240 stars 97 forks source link

ENH: Add Travis #94

Closed larsoner closed 10 years ago

larsoner commented 10 years ago

Travis builds pass, see below, and coveralls has coverage info.

Getting Travis to work required moving the example_data (~2.8 MB) into the package (as surfer/data), which I think is a welcome change anyway. I also fixed a few pep8 issues.

Also, the fsaverage_min.tar.gz file is currently hosted on my space at UW, which is not a tenable long-term solution. @mwaskom do you have somewhere you could host it? If not, @agramfort would it be possible to upload it to Martino's space somewhere? It's <30MB.

larsoner commented 10 years ago

Correction, I mean the build now passes:

https://travis-ci.org/Eric89GXL/PySurfer/jobs/18246245

larsoner commented 10 years ago

FYI I was able to activate the service hook for Travis (I guess I have enough permissions to do so?). But I don't think I can setup coveralls, though.

larsoner commented 10 years ago

I think I got coveralls working, see:

https://coveralls.io/r/nipy/PySurfer

So now we just need to sort out where to host the fsaverage_min.tar.gz file, and this should be good to go.

agramfort commented 10 years ago

nice

I am just a bit -1 on the “surfer/data". It was nice to have the data for the examples in the examples data.

also the surfer/data should not be installed with pip. Check manifest.in

larsoner commented 10 years ago

Okay, I cam change that back

mwaskom commented 10 years ago

I never thought I'd see the day. This is awesome!

As for the examples, I'd just keep in mind that it seems a common thing for beginners to copy the examples (I guess from the website) and try to run them, then modify for their specific task. So we should make sure the changes aren't going to interfere with that process/confuse people who give no thought to the constraints of CI, etc.

If people pip install PySurfer and then try to use surfer.data_path are they going to get a confusing error? I might lean towards using a function that makes it a little bit more explicit that you're trying to fetch built-in data, with more informative failure modes when the data don't exist locally.

larsoner commented 10 years ago

@mwaskom I think I'll just revert the changes for now, leaving reorganization for another PR if necessary (probably not)

larsoner commented 10 years ago

Okay, surfer/data changes reverted, should be ready for review @mwaskom @agramfort

agramfort commented 10 years ago

perfect

@mwaskom please merge if you're happy

thanks @Eric89GXL !

mwaskom commented 10 years ago

Very happy to merge, this is awesome.