Add notebook-gallery plugin to auto-generate gallery style cards on index pages#298
Add notebook-gallery plugin to auto-generate gallery style cards on index pages#298jaladh-singhal wants to merge 4 commits intoCaltech-IPAC:mainfrom
notebook-gallery plugin to auto-generate gallery style cards on index pages#298Conversation
Add packaging to it since we need yaml npm package
Add tests for exception handling
|
This is relevant for build step changes in tox: https://app.circleci.com/pipelines/github/Caltech-IPAC/irsa-tutorials/849/workflows/b6cd0cd5-fd7d-43d6-9d30-86bd8a2c1fc6/jobs/826?invite=true#step-103-10522_83
Also note L265: the plugin is loaded successfully (right now we just have 1 directive and that's what it tells us) |
bsipocz
left a comment
There was a problem hiding this comment.
This looks great, thank you so much.
I think 1) this should go in as first version before we jump into with new ideas for improvements (with the removal of the test sections of course -- those could maybe go into a new test md file in the plugin itself, or added as a test md file if/when the plugin will live in its separate repo)
| plugins: | ||
| # note: we need to use the built plugin bundle here so make sure to build it with | ||
| # `cd plugins && npm install && npm run build` after any changes to the plugins/src/ | ||
| - plugins/dist/notebook-gallery.mjs |
There was a problem hiding this comment.
This is OK for now, but ultimately we may want to ship the plugin separately from this repo.
There was a problem hiding this comment.
I agree - and note the plural in "plugins", I envision it as a suite of plugins that we can maintain, ship and potentially share with MyST community.
There was a problem hiding this comment.
I am definitely thinking of multiple plugins, whether to ship them one per repo or multiple in a plugins one is a separate question :)
| buildhtml: bash -c "echo 'Notebooks ignored (not tested/executed) in this job:'; cat ignore_execute | sort | uniq" | ||
|
|
||
| # Build the MyST plugin bundle before building the site with jupyter-book | ||
| buildhtml: bash -c "cd plugins && npm install && npm run build" |
There was a problem hiding this comment.
It's OK here, but we will need to make sure npm is available for tox? I suppose we can kind of rely on that it's available already for JB so we can just piggy-back?
There was a problem hiding this comment.
Yes, it seemed to work locally as well as on the CircleCI/GA runner had npm pre-installed (which is common for most of their runner images). I don't think we particularly piggybacked on JB because we install JB via pip as python package not as an npm package.
That being said, I didn't think this through and we should be aware of it if we run into npm missing issues. Another reason why plugins/ should be it's own repo and we don't maintain the build step here at irsa-tutorials.
|
|
||
| # Using srtict so we fail with trackbacks and debug mode to have a richer log | ||
| # For full build we disable parallel runs to easy server load | ||
| buildhtml: bash -c "jupyter-book build --execute --execute-parallel 1 --html --strict -d" |
There was a problem hiding this comment.
If this goes in with the merge we will need to open a follow-up issue to ensure that eventually we turn back on the checks.
There was a problem hiding this comment.
I will revert it before merging.
The first build failed (when strict was on) not because of plugin installation step but because of some notebook or something (I couldn't tell because the logs are way more verbose than my attention span). But yeah, I'm afraid it's becoming more common to remove strict for these kind of PR builds, maybe we should make it a label like skip testing GH label?
There was a problem hiding this comment.
Haha, yes, we do use debug level logging to have more info available when there are issues.
Ideally no content executing testing was done with these changes as you haven't touched the notebooks themselves; so I suppose we should go back and check what was going on with that failure.
| ```` | ||
| ```{notebook-gallery} notebook_metadata.yml | ||
| tutorials/euclid/1_Euclid_intro_MER_images.md | ||
| tutorials/euclid/3_Euclid_intro_1D_spectra.md |
There was a problem hiding this comment.
I like this approach very much; thanks Jaladh.
However; I already start seeing a list of options for improvements. What would work best, comment on the plugin itself or here argue for the usecase?
There was a problem hiding this comment.
I mean it's upto you. If you wish to get this merged, then you can open an issue listing all improvements you see. Else you can comment here and I can iterate within the PR
@bsipocz Agreed. I plan to add a readme when it becomes more mature and independent. That readme will be a good place for these test cases. |
|
OK, so features that comes to mind, but I don't think we should wait for any of them and just get this PR merged as is already:
|
troyraen
left a comment
There was a problem hiding this comment.
This is great, thank you!

Fixes #233
Added a custom local MyST plugin (
notebook-gallerydirective) that auto-generates a notebook gallery we have for the Euclid index page and used it for all other mission pages.Design decisions:
@bsipocz you probably care about this more (since we had past conversations about this):
.mjsfile) rather than a Python script, as the MyST plugin system integrates JS plugins cleanly at build time and avoids a separate generation step for executable plugins. Also if we want more complexity we can easily migrate to widgets from JS route - right now we don't need it because we are not working with HTML DOM directlyyamlnpm package for parsingnotebook_metadata.yml. So a build step is required and I had to addpackage.json(+package-lock.json) and a bundling step viaesbuildintox.ini(as opposed to a single self-contained.mjsfile).plugins/to a separateirsa-myst-pluginsrepo and publish it as npm/GH package if we decide to reuse it across projects or to contribute upstream.Things to check:
euclid.mdwill be removed before merging