search

reproducible-research / scipy-tutorial-2014

Tutorial on reproducible research practices at SciPy 2014
http://reproducible-research.github.io/scipy-tutorial-2014/
Apache License 2.0
29 stars 95 forks source link

ADD notebooks, not mount #18

Closed jhaubrich closed 10 years ago

jhaubrich commented 10 years ago

Working in OS X.

I removed the stdin stuff in build-and-run.sh which provided the proper context boot2docker needed.

I also manually copied notbook files into the ipython build. Couldn't get -v to work properly.

You won't be able to compile documents with dexy, but will be able to follow along with the notebook.

jcfr commented 10 years ago

Thanks for your contribution :+1:

That is a rather large Pull Request and it would be preferable to create multiple PR where each one would address a specific problem.

That would make review and integration easier.

I will let @thewtex and @ananelson comment on the specific related to the docker env and dexy changes.

ananelson commented 10 years ago

Please separate into multiple PRs. Also commented out code is not suitable for PRs. Using ADD is a possible alternative, but it then means that changes to notebooks are not saved.

jhaubrich commented 10 years ago

I loved the tutorial!

This isn't a quality pull request. I submitted as soon as I got it working so that other people in the tutorial session could see it and get their environment up. I ended up forking and just supplying that to others during class.

As for the size of the PR -- There are actually only a few lines changed in the dockerfile and build script. The rest of the commit is relocating the notebooks; which solved the problem of volume mounts not functioning on some of our machines.

This patch does the following:

  1. Removed the stdin stuff from the build script, which is apparently buggy and fails to provide the proper context in some cases. @thewtex, docker build - couldn't find the references in the Dockerfile. This was the error
  2. -v failed to mount volumes on our macs, so the ipython notebooks were moved inside the docker images (this may be what caused the above error, I can't remember).
  3. User permissions were messed up inside the docker images home directory. Ipython notebook did not have write permissions to make checkpoints or save files. The repro user was removed.

Again sorry for the quality of this pull request. I ended up making more changes in the fork I was passing around.

An aside, you might be interested in implementing data volumes. You would be able to have both dockers running with the same mounted volume, and the doxy-docker could watch the ipython notebooks for changes, and compile automatically on saves.

Cheers!

thewtex commented 10 years ago

@jhaubrich thanks again for the feedback!

I've collected a few of the critical changes here:

https://github.com/reproducible-research/scipy-tutorial-2014/pull/21

Thanks for the suggestion on not using stdin for the Dockerfile input.

Unfortunately, the stability of Docker on the Macs was not as good as I would have expected, but I am sure that will improve in good time. The data volumes are a good suggestion. They were left out because it may have been overwhelming for the tutorial. But, using them is a best practice and it would have helped many of the Mac folks who had to SSH in with 'boot2docker ssh', didn't know how to get results out of the VM, and there was not persistence in the VM.