search

karthik / binder-test

15 stars 7 forks source link

Feedback on tutorial #1

Closed annakrystalli closed 5 years ago

annakrystalli commented 5 years ago

Firstly, really nice straight forward demo! đź‘Ź Here's a couple of points for your consideration:

Capturing versions of dependencies

One thing that’s “maybe missing” is a runtime.txt file, but then I’ve also heard that using a Dockerfile means that runtime.txt is ignored? Indeed this is my main conceptual hole about the Dockerfile + DESCRIPTION approach currently, ie how can we capture specific versions of dependencies? I've tried the package (== version) approach in the DESCRIPTION but that didn't seem to work. This was actually on my top priority to try and figure out so if you have more info on this it would be great to hear!

Launching binder

Clicking the binder button: works like a treat

build_binder(): Am not sure if it works. I’m somewhat unclear at what exactly the function is supposed to do (some detail in the function documentation could help users know what to expect). I’m guessing it’s launching a binder instance but is it supposed to also launch the browser when it’s done? If so it isn’t for me.

It does print out info, including various urls and a token, but it’s unclear what if anything one should do with this. I tried various urls given but the $url in the list printed just leads to a Binder inaccessible error page while the final url the function prints out (https://mybinder.org/build/gh/annakrystalli/testdrive-holepunch/master) just contains the details printed above it.

Minor suggestions

karthik commented 5 years ago

Thanks Anna! This feedback is great.

Am not sure if it works. I’m somewhat unclear at what exactly the function is supposed to do (some detail in the function documentation could help users know what to expect). I’m guessing it’s launching a binder instance but is it supposed to also launch the browser when it’s done? If so it isn’t for me.

Excellent question. I really need to document this. Build binder kicks off the build for the first time and creates an image on binder. This step can take 30 seconds to several minutes (or longer) if someone were to do this for the first time by clicking the badge. Doing this in the console means that it will get built and be ready for a quick launch when someone clicks the badge. Right now I show the streaming json output, but it can be slow and doesn't need to hold up the console. So I will clarify this on the docs and just say it's good practice to kick off the build so that it's ready for a fast launch when you want it.

It can also launch the browser when ready, but it is currently not set up yet. Would that be helpful?

Would be good to include a sentence on what the analysis relates to and show the plot so they have something to compare the plot they are getting in their binder and satisfy themselves that everything is working (and of course, everyone loves a pretty plot!).

Excellent point. I'm still on the hunt for a nice clean analysis that tells a short but good story, and involves several packages beyond the tidyverse. Do you have ideas? This was just something I found while searching but I need something better.

Would also be good to get participants to complete the DESCRIPTION file (to get the hang of it and get them into the habit of doing it.)

Great! It is on the TODO list but I will open a separate issue to get this addressed in the documentation.

karthik commented 5 years ago

One thing that’s “maybe missing” is a runtime.txt file, but then I’ve also heard that using a Dockerfile means that runtime.txt is ignored?

Correct.

Indeed this is my main conceptual hole about the Dockerfile + DESCRIPTION approach currently, ie how can we capture specific versions of dependencies? I've tried the package (== version) approach in the DESCRIPTION but that didn't seem to work. This was actually on my top priority to try and figure out so if you have more info on this it would be great to hear!

This is also in the TODO but I will ask for your feedback in a issue on the holepunch repo.

annakrystalli commented 5 years ago

Build binder kicks off the build for the first time and creates an image on binder. This step can take 30 seconds to several minutes (or longer) if someone were to do this for the first time by clicking the badge. Doing this in the console means that it will get built and be ready for a quick launch when someone clicks the badge.

Aha! Makes sense and the reduction in launch time when clicking the button is very noticeable. I'd say launching the browser with the option to suppress it would be a nice feature (or vice versa).

Excellent point. I'm still on the hunt for a nice clean analysis that tells a short but good story, and involves several packages beyond the tidyverse. Do you have ideas?

Not off the top of my head sadly but I'll keep an eye out.

karthik commented 5 years ago

build_binder(): Am not sure if it works. I’m somewhat unclear at what exactly the function is supposed to do (some detail in the function documentation could help users know what to expect). I’m guessing it’s launching a binder instance but is it supposed to also launch the browser when it’s done? If so it isn’t for me.

It does print out info, including various urls and a token, but it’s unclear what if anything one should do with this. I tried various urls given but the $url in the list printed just leads to a Binder inaccessible error page while the final url the function prints out (https://mybinder.org/build/gh/annakrystalli/testdrive-holepunch/master) just contains the details printed above it.

This is now fixed (although there is room for improvement, for e.g. running this in the background). Once finished, it will launch the browser to the notebook.