ml-evs / matador

⚗️ matador is an aggregator, manipulator and runner of first-principles calculations, written with a bent towards battery 🔋 electrode materials.
https://matador-db.readthedocs.io
MIT License
29 stars 19 forks source link

Binder notebooks do not work #121

Closed srmnitc closed 3 years ago

srmnitc commented 4 years ago

This is part of the review at openjournals/joss-reviews#2563

There are quite a lot of examples, which is really nice. I was trying to run the examples on mybinder, and I get the feeling that they are not really optimized to run on a binder instance. The binder instance is a good way to get started with matador, and I strongly recommend that these notebooks be made a bit more user friendly. I have summarized the issues below-

I completely understand that it is difficult to maintain numerous examples such as this, but since it provides a good entry point for the potential user. The notebook should be completely executable unless otherwise specified.

ml-evs commented 4 years ago

Thanks for this thorough review @srmnitc.

Many of these problems are related, e.g. many of the warnings on the notebooks in your second point relate to missing fonts in the Binder instance.

My suggested fix is to make another folder /interactive_examples to point the Binder link to, which will contain symlinks to the notebooks that are actually interactive without extra data.

I'll also go through and add some more clarifying descriptions to the notebooks themselves.

srmnitc commented 4 years ago

Thanks @ml-evs! I completely agree that separating them to a folder would be a good solution.

ml-evs commented 3 years ago

Hi @srmnitc, I think #125 closes this now.

I moved split the examples into interactive/non-interactive in the online documentation, added a higher level summary in the examples folder, and fixed the dependencies that were being installed under myBinder.

Unfortunately I'm struggling to get myBinder to launch the image right now (it builds fine, but runs into miscellaneous launch errors) but I'll try again in the morning.

Once again, please feel free to reopen this if you are not satisfied.

srmnitc commented 3 years ago

The binder instance still seems to have problems. But everything overall seems to be great!

ml-evs commented 3 years ago

The binder instance still seems to have problems. But everything overall seems to be great!

Yes, you're quite right, I've been trying to fix it in #130. Turns out that the Dockerfile approach is not well-supported. I just need to do a final check of every notebook to make sure they work with the new filepaths, then it should be good to go.

ml-evs commented 3 years ago

That seems to have done it!