Add notebooks to docs download #652
Conversation
Co-authored-by: Penelope Yong <penelopeysm@gmail.com>
…into add_notebook_dl
…into add_notebook_dl
|
I think this partially satisfies some of the points from #641 ~ it might be worth adding some note about these being available in the docs / have a quick way to export into online systems, as discussed in that issue. |
assets/scripts/qmd_to_ipynb.py
Outdated
| def __init__(self, qmd_path: str): | ||
| self.qmd_path = Path(qmd_path) | ||
| self.cells: List[Dict[str, Any]] = [] | ||
| self.kernel_name = "julia-1.11" # Default kernel |
There was a problem hiding this comment.
Generated notebooks don't work in Google Colab, or am I doing something wrong? I think the defined kernel might be the issue since Colab only supports the lts version of Julia by default. Also, we'll need to change this hardcoded version every time we update the docs to a new version. Maybe we should add a note in the README or change it to use the version from an env file or some variable that we can use everywhere.
There was a problem hiding this comment.
I think latest push should fix this. Lemme verify and confirm once it builds
|
Notebooks work fine with |
penelopeysm
left a comment
penelopeysm
left a comment
There was a problem hiding this comment.
Overall thoughts:
-
Looks very nice! I like the little icon and it fits in thematically with the rest of the website very nicely.
-
When you download the notebook, the first line is
using Pkg; Pkg.instantiate(). I attempted to run this in a VSCode and this loads the global Julia environment. So if you subsequently runusing Turingthat will also add Turing to the global environment, which is probably not ideal for users.
Is it possible to modify the notebook generation such that the very top of the notebook contains a new code block saying using Pkg; Pkg.activate(; temp=true), before the call to instantiate()?
Yes! I'll need to modify the new file added |
assets/scripts/add_notebook_links.sh
Outdated
| if ! grep -q 'Download notebook' "$html_file"; then | ||
| # Insert the notebook link AFTER the "Report an issue" link | ||
| # This ensures it goes in the right place in the sidebar toc-actions | ||
| # The download="index.ipynb" attribute forces browser to download instead of navigate | ||
| perl -i -pe 's/(<a href="[^"]*issues\/new"[^>]*><i class="bi[^"]*"><\/i>Report an issue<\/a><\/li>)/$1<li><a href="index.ipynb" class="toc-action" download="index.ipynb"><i class="bi bi-journal-code"><\/i>Download notebook<\/a><\/li>/g' "$html_file" |
There was a problem hiding this comment.
Could you make the string 'Download notebook' a variable?
|
Overall very excited about this, very happy that I clicked the download link and it just worked (tm) :) |
|
Looks good -- it is very helpful to add an "Open in Colab" button too. URLs like will automatically open a notebook from the given Github repo. |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
penelopeysm
left a comment
penelopeysm
left a comment
There was a problem hiding this comment.
Just a parsing detail, otherwise happy.
| # Handle 'using Package: item1, item2' format - extract just the package name | ||
| if ':' in remainder: | ||
| package = remainder.split(':')[0].strip() | ||
| if package and package != 'Pkg': | ||
| self.packages.add(package) | ||
| else: | ||
| # Handle 'using Package1, Package2, ...' format | ||
| packages = [pkg.strip() for pkg in remainder.split(',')] | ||
| for pkg in packages: | ||
| if pkg and pkg != 'Pkg': | ||
| self.packages.add(pkg) |
There was a problem hiding this comment.
There's another edge case to handle, which is using Foo.X.Y where the package name is just Foo and not Foo.X.Y.
There's also import as well as using.
This was to try and fix #642 as I was afraid of a caching issue, please see that PR for previous discussions