choderalab / msm-pipeline

A pipeline for MSMs.
GNU Lesser General Public License v3.0
2 stars 5 forks source link

[WIP] Add Travis #9

Closed maxentile closed 8 years ago

maxentile commented 8 years ago

Should add more meaningful tests, but for now this just tries to run test_report_generation.py.

Not sure why build is currently failing though. Travis error says MDTraj requires Cython, but Cython is definitely listed in requirements.txt.

Error: building mdtraj requires numpy and cython>=0.19

Try running the command ``pip install numpy cython`` or ``conda install numpy cython``.

Will probably be simpler to use conda here.

jchodera commented 8 years ago

Thanks for taking a stab at this!

To reduce technical debt, I am strongly in favor of organizing this repo in the same manner that essentially all our other tools are organized. Currently, your approach uses a completely different approach, and I think that will only make it more confusing for people in the lab.

You can see lots of other projects that use the standard scheme:

It would be good to follow this model. I'm happy to set it up if you prefer, or you can take a stab at it if you want to give it a try!

The basic structure of the repo is more or less:

jchodera commented 8 years ago

It's also essential to have your Python scripts organized inside of a package that gets installed via a setup.py.

You can see how this works for FAHMunge, where the Python is all inside a fahmunge/ subdirectory and gets installed via setup.py, and the entry_points block of the setup.py is used to create a script called munge-fah-data that actually starts the function main() in fahmunge.cli. The entry points have a lot of flexibility, so you can use this to call into your python scripts in many different ways.

maxentile commented 8 years ago

Thanks, John!

I had initially just put barebones scripts here, but I think that was a sloppy / confusing choice on my part, and I agree it will be better to use the same model as the lab's other tools.

Shall I merge the Clean-up PR, then start structuring the repo according to the lab's standard scheme in a new PR?

jchodera commented 8 years ago

Shall I merge the Clean-up PR, then start structuring the repo according to the lab's standard scheme in a new PR?

Sounds like a great plan!

It's better to bite the bullet and make these changes early in the project, rather than late. The benefits of travis testing, conda deployment, and standard repo layout really make up for the time investment in getting these things set up initially (which is usually about an hour per repo).

sonyahanson commented 8 years ago

Thanks for the first comment here John, this is very useful. Once this is finished we can close #4, too, which is somewhat sparse as I was not sure msyelf of the ideal structure.

sonyahanson commented 8 years ago

I had to pip install corner to get things to work. It doesn't seem like this is currently included in this PR. Could this by why tests are failing?

Though I guess this doesn't help explain the travis error...

sonyahanson commented 8 years ago

How are we doing with this? I think @maxentile was going to take a stab at this, and if it didn't seem doable in a timely manner @jchodera would 'do it in half an hour'.

maxentile commented 8 years ago

Oh shoot! So sorry for dropping the ball here.

Will open a new PR with the standard structure in a moment...