Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding download links for the notebook files #104

Merged
merged 3 commits into from
Feb 11, 2019

Conversation

choldgraf
Copy link
Collaborator

This riffs off of #38 which @TomDonoghue pioneered getting a download link to work.

In that PR we were using a Zip file to download because it seemed like that was the only way to get the file to "auto" download. I found a blog post (https://www.philowen.co/blog/force-a-file-to-download-when-link-is-clicked/) that showed how to make the file automatically download even if it's an ipynb file. So, this PR does two things:

  1. Adds @TomDonoghue 's commit that adds the download + zip functionality
  2. Adds one more commit that strips some of this away and uses the interact link path to directly download the notebook file.

I still think that it could be a good idea to use zip files, but for now since we were only downloading the single notebook file anyway, this seems like a much simpler path that we can build upon. @TomDonoghue what do you think?

@@ -0,0 +1,4 @@
{% if site.use_download_button -%}
<a href="{{ page.interact_link | relative_url }}" download>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomDonoghue apparently the download> part here is what causes the browser to auto-download instead of trying to open the file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's cool - nice find!

@choldgraf
Copy link
Collaborator Author

an example page w/ a download link: https://640-137292803-gh.circle-artifacts.com/0/html/features/notebooks.html

@@ -0,0 +1,4 @@
{% if site.use_download_button -%}
<a href="{{ page.interact_link | relative_url }}" download>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's cool - nice find!

@@ -1,5 +1,4 @@
"""A helper script to "clean up" all of your notebooks and
generated markdown files."""
"""A helper script to "clean up" all of your generated markdown, zip, and HTML files."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had updated this comment, since at the time I was 'cleaning' out created zip files, but as is, the reference to zip files is outdated.

@@ -9,6 +9,7 @@
from tqdm import tqdm
import numpy as np
from glob import glob
from zipfile import ZipFile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is currently not used - can be dropped.

@TomDonoghue
Copy link
Contributor

TomDonoghue commented Feb 11, 2019

@choldgraf - thanks for porting this to the new organization!

I like the update to just grabs the notebook file itself - nice and clean. Couple small comments on the code above.

There do appear to be some os / browser specific characteristics. When I download the the file from the example you linked above, browsers seem to recognize the file as a txt file. This is broadly fine, except that for some reason Safari then appends a '.txt' to the downloaded file, which is annoying.

I think Safari just in general has different approaches. It also, by default, opens zip files, so in the zip version, Safari ends up with the notebook file. On my system (MacOS), Firefox & Chrome are both more hands off - downloading the unzipped files normally (as .ipynb) and keeping the zip files as zip.

^I'm not sure that there's anything to be done about this, just worth knowing I guess. Possibly downloading zip files does have a more consistent effect, than possible variation of how .ipynb files are treated?

Also - presumably, we have a similar desired workflow to how sphinx-gallery downloads notebooks and files. From a quick scan, they don't seem to zip by default for single files, but something is different which avoids the file extension problem in safari, for me at least. Haven't had a chance yet to figure out how they're doing it, but maybe worth looking into.

@choldgraf
Copy link
Collaborator Author

Thanks for the feedback @TomDonoghue !

I cleaned up the extra text you flagged, and also added a short section in the FAQ about behavior on different browsers. Gonna merge this one and start iterating from there!

@choldgraf choldgraf merged commit 5b48156 into jupyter-book:master Feb 11, 2019
@TomDonoghue
Copy link
Contributor

All looks good! Thanks for finishing this up & merging @choldgraf

choldgraf added a commit to choldgraf/jupyter-book that referenced this pull request Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants