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

DOC: Configure JupyterLite's TryExamples directive for UNU-RNS tutorial #5

Merged

Conversation

agriyakhetarpal
Copy link

@agriyakhetarpal agriyakhetarpal commented Mar 27, 2024

Reference issue

See scipy#20303 for more information

What does this implement/fix?

This PR addresses the following changes:

  1. adds jupyterlite-pyodide-kernel as a dependency to debug changes to Examples enabled with jupyterlite-sphinx,
  2. wraps all code blocks in the sampling.md file with {eval-rst} to let jupyterlite-sphinx correctly interpret them and parse code snippets from them, and
  3. temporarily enables these examples for debugging purposes.

Additional information

N/A

Comment on lines +196 to +197
>>> from scipy.stats.sampling import TransformedDensityRejection
>>> rng = TransformedDensityRejection(dist, domain=(-1, 1), random_state=urng)
Copy link
Author

Choose a reason for hiding this comment

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

Some of these import statements will be required in these code snippets to make sure that the button works and the examples run without issues for users. I'm not sure if there is a better or cleaner way to do this, because it will create repeated imports (though running them won't be a problem because imports are cached)

@agriyakhetarpal
Copy link
Author

Hi, @melissawm! I'm not sure if this is right and addresses all of the concerns we have, but it is an almost complete fix. The only example which hasn't been converted yet on this page is the histogram plot that gets displayed under the

```{code-cell}
---
mystnb:

directive, and I'm not sure if it can be converted to a JupyterLite-enabled example at this time because of the alt-text being rendered in the image. However, adding the following:

```{eval-rst}
.. try_examples::
.. plot::
    >>> import numpy as np
    >>> import matplotlib.pyplot as plt
    >>> from scipy.stats import norm
    >>> from scipy.stats.sampling import TransformedDensityRejection
    >>> from math import exp
    >>>
<!-- the rest of the plotting example here -->
```

i.e., not converting to pure Markdown seems to work and displays the plot as needed, but I'm unaware if MyST-NB then chooses to ignore this example or runs it, because there are no additional warnings with this approach and the documentation proceeds to pass – with the JupyterLite example working as well.

Another thing we could try is using the rst-to-myst CLI tool to convert the file from .rst to .md as needed, which would be compliant with Sphinx too and would (likely) help us retain MyST-NB's functionality. Also, I would request you to please excuse my inexperience with MyST-NB, having used nbsphinx a lot more over the years :)

@melissawm
Copy link
Owner

melissawm commented Apr 3, 2024

@ev-br had a good suggestion at the community meeting just now: what if instead of turning jupyter-lite on for each code block, we turned it on for the full notebook? Would that work? (I know we still wouldn't have the outputs necessarily, but I think that would be easier and we would also not need to repeat the import statements between code-blocks, right?)

@mdhaber
Copy link

mdhaber commented Apr 3, 2024

I had assumed it would be turned on for the whole notebook. I'd prefer that.

@agriyakhetarpal
Copy link
Author

agriyakhetarpal commented Apr 4, 2024

I'm not sure if we can turn it on for the full notebook – there is a NotebookLite directive that we can use, but it works for notebook files (.ipynb), not Jupytext notebooks like this one. So, I have two questions/comments:

  1. Would converting the notebook to sampling.ipynb be a step backwards, considering that something like nbmake would be needed to be configured to test the notebook?
  2. Another method we can tackle this could be to convert this notebook from Jupytext to ipynb, but only at the time of building the docs so that one can access it through the NotebookLite directive (and then we will have both the Jupytext notebook in this .md file, + a button at the end of the page that can open the notebook in an activate kernel). A small Sphinx extension can do this, probably – it would just need to connect to the right event, which should be an event before jupyterlite-sphinx connects so that the .ipynb files to load would be available. I think one can avoid the complexity of creating a Sphinx extension though and just run a converter script before building the docs altogether. How does this sound?

Edit: I remembered from the above PR that this PR is merging to:

I don't know what would be the ideal solution here right now. For the future, we could think of a hybrid option that would provide executed notebooks and still have the try with jupyterlite button, but I'd like to propose decoupling that from this PR and investigating further.

my second point sounds (a bit) like this method – it is a hybrid option that can provide executed notebooks (the execution would get handled by MyST-NB as is) and therefore outputs of code cells, the jupytext CLI can be executed to create .ipynb versions of these notebooks, and Sphinx can just render the Jupytext notebook plus a button at the end (which we can add manually) that opens the IPyNB notebook for interactive use. I think the Jupytext converter should be fairly stable since it has had a major v1 release (v1.16 at the time of writing) so the conversion should be reliable enough without any data loss in an ideal scenario.

Edit 2: oops, Jupytext isn't bidirectional I think – it can convert .ipynb to other formats, but not Markdown to .ipynb – in that case, maybe nbconvert would be good to try? I am not sure how fast it will be, though, so we should probably take the docs build time into consideration as well; do we have a quote on that for the SciPy docs, @melissawm? The PyData Sphinx Theme had forced a serial write recently (pydata/pydata-sphinx-theme#1643) but as it turns out it is only the SciPy docs that seem to be affected, so something around the ten-minute range would be a good estimate? I can build in a minute in serial mode, but without the doctrees cached it takes around seven minutes, in CI it should be a bit more slower.

@melissawm
Copy link
Owner

melissawm commented Apr 4, 2024

@agriyakhetarpal fortunately that already happens! When MyST-nb runs through and executes the md files, there is an intermediate step that generates ipynb files that can later be linked to (see for example this page, https://jupyterbook.org/en/stable/file-types/notebooks.html - one of the buttons at the top right of the page allows you to download the page as an ipynb.) This can be done - however recovering this link from the sphinx context is very tightly coupled with the jupyterbook theme and I was trying to avoid doing that for now because it's not necessarily in scope for the work that I'm doing. If you have a different solution though, maybe we can try? Thanks for the help!

@agriyakhetarpal
Copy link
Author

agriyakhetarpal commented Apr 5, 2024

Thanks for the additional info, @melissawm! If it's tricky to extract the links, I think we'll still have as a solution the nbconvert pathway, but with that we'll be duplicating the execution of the notebooks. It might take a bit of time later on if we'll have all of the notebooks converted, but if there won't be more than ten or twenty or so then it could be okay (we can stop it for PR builds to speed them up, by conditionally checking for environment variables or something?)

However, if converting other notebooks to this format is the long-term plan, maybe we can keep these notebooks saved after execution, such as with the use of nb_execution_mode = "cache" instead of the currently-set auto and construct links to those files from there (just a glob pattern could work)? I haven't used MyST-NB much before, so I am not sure if performs a 1:1 map of these and preserves the names for the notebooks or if it mangles the names

@melissawm
Copy link
Owner

I think it may be simpler than that - let me try and take a look again. Thank you again so much - the help is really appreciated!

@melissawm
Copy link
Owner

Ok, here's a proposal, and let me know if this is reasonable.

As a compromise, we would be happy with having a button on the notebook that says "Open this file as a notebook on JupyterLite". When clicking this button, we could open this notebook in a new tab with the existing SciPy JupyterLite kernel.

I am not sure how to accomplish this with the jupyterlite-sphinx extension or if this is even the right approach, but I did manage to get an embedded jupyterlite instance with the jupyterlite directive:

https://melissawm.github.io/scipy/tutorial/stats/sampling.html

There are two problems remaining:

  • Ideally, we would not have an embedded notebook, but a new JupyterLite tab;
  • We would also need this new JupyterLite tab to open the sampling.md file as sn executable notebook. It doesn't look like this works right now. Despite me using the directive like so:
```{eval-rst}
.. jupyterlite:: sampling.md
   :width: 100%
   :height: 600px
   :prompt: Try JupyterLite!
   :prompt_color: #00aa42
```

what I get is a jupyterlite window with many many files shown; if you scroll all the way to the bottom of the file list you can indeed find sampling.md. If you right click on it and select "Open as Jupytext Notebook" there is a 404 error:
Screenshot_20240407_120825

Is this reasonable? I'd be happy to jump on a call and talk about it if it helps. Thanks!

@mdhaber
Copy link

mdhaber commented Apr 7, 2024

As a compromise, we would be happy with having a button on the notebook that says "Open this file as a notebook on JupyterLite". When clicking this button, we could open this notebook in a new tab with the existing SciPy JupyterLite kernel.

If this means that the tutorial would initially render as static text, then clicking the button would open the whole tutorial in a new tab as a notebook, this sounds great.

@melissawm
Copy link
Owner

That's the idea, yes. I think this would be the best of both worlds, as we could keep the outputs correctly rendered in the main page and then, for those looking for the interactivity, this would be a deliberate choice. They would also be able to download the notebook from this new tab.

@melissawm
Copy link
Owner

I will merge this and keep working on the main PR according to our discussion @agriyakhetarpal - thanks again!

@melissawm melissawm merged commit c1c6cf2 into melissawm:stats-tutorials Apr 8, 2024
@agriyakhetarpal agriyakhetarpal deleted the agriya/stats-tutorials branch April 8, 2024 21:02
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.

3 participants