PyWavelets / pywt

PyWavelets - Wavelet Transforms in Python
http://pywavelets.readthedocs.org
MIT License
1.97k stars 460 forks source link

Initial steps towards interactive documentation via JupyterLite #728

Closed agriyakhetarpal closed 3 months ago

agriyakhetarpal commented 3 months ago

Description

This PR adds interactive documentation via JupyterLite Pyodide-enabled kernels and tests them on Read the Docs.

The key changes here are:

  1. It enables the JupyterLite Pyodide kernel and the JupyterLite Sphinx extension for the documentation, for both building locally and for the hosted documentation on Read the Docs (this can be seen in the PR previews).
  2. It enables the JupyterLite extension for all of the doctest-based examples in the API reference wherever applicable, and adds a warning at the top of the notebook to warn users about how experimental these changes are.
  3. The style guidelines have been mimicked from those for SciPy, through this PR: https://github.com/scipy/scipy/pull/20019

Following this, users shall be able to run all of the examples by loading an installation of PyWavelets in notebooks inside the documentation, which can be opened in new tabs too, as necessary.

Footnotes

This is meant to address certain sections of gh-706, however, further follow-ups are required to enable interactivity for the rest of the available examples.

agriyakhetarpal commented 3 months ago

The RTD PR preview fails to appear here, @rgommers – could you please help debug?

rgommers commented 3 months ago

The RTD PR preview fails to appear here, @rgommers – could you please help debug?

It did actually build: https://pywavelets--728.org.readthedocs.build/en/728/. I'll check why there's no entry in the list of CI jobs here.

agriyakhetarpal commented 3 months ago

Thanks for the link! I'll use that for now and for future PRs. It is a bit strange, but as a workaround, we can use this GitHub Action: https://github.com/readthedocs/actions/tree/v1/preview if needed. It looks like it is a pretty easy to configure?

rgommers commented 3 months ago

I added an integration. Not too happy with how many permissions the RTD Oauth app wants, but all right - let's see if that worked.

agriyakhetarpal commented 3 months ago

I added an integration. Not too happy with how many permissions the RTD Oauth app wants, but all right - let's see if that worked.

Thank you! It does show up here now – we can switch to the PR preview action at any time, if all we need is just a link. It will require write permissions to edit the PR description.

agriyakhetarpal commented 3 months ago

The Wavelet object example that you mentioned on Slack does not seem to be working – but I think that is because it's configured incorrectly (it is placed in the documentation in a .. sourcecode:: directive).

On the same page, other basic examples, i.e., those under an

Examples
--------

section and elsewhere are working wherever this heading is mentioned in the docstring, for example – this is for listing down the different types of Wavelet families available in PyWavelets. The size of the kernel is a tad too small – I'll configure that with other aesthetic changes, such as the size and shape of the "Try it" button(s).

The code snippet does take a bit to load, and did not work unless I opened it in a new tab (maybe I have way too many open tabs or something?) This example does work, and I confirmed via

import pywt
print(pywt.__version__)

that we have 1.4.1 being loaded currently. So, my immediate next step will be to configure how to import plus install the nightly WASM wheel, as noted in the links under this section.

rgommers commented 3 months ago

The size of the kernel is a tad too small – I'll configure that with other aesthetic changes, such as the size and shape of the "Try it" button(s).

This is still WIP, right? It looks like this:

image

compared to in SciPy (which uses the same theme and plugins):

image

So, my immediate next step will be to configure how to import plus install the nightly WASM wheel, as noted in the links under this section.

Can you do that in a follow-up PR? This should be merge-able while using the PyWavelets version shipped by Pyodide.

agriyakhetarpal commented 3 months ago

Yes, I shall improve the styling here – there are a couple of guides available about this in the JupyterLite docs, or I can follow the footsteps of the SciPy docs and re-use a similar style narrative.

Can you do that in a follow-up PR? This should be merge-able while using the PyWavelets version shipped by Pyodide.

I personally don't think it is a good idea to merge this on the development version of the documentation when we do not have the nightly wheels set up, but a suitable workaround for now could be to make a note about this to users in the currently added admonition – referencing that the version of PyWavelets available may be a bit outdated and therefore some of the examples might not work?

I will make and verify some additional changes to ensure that the button is available on all of the examples running under the doctests. Do we need to add the button to the Usage examples too, or just these API reference examples would be enough for this PR?

rgommers commented 3 months ago

There are no new functions in the 1.5.0 and 1.6.0 releases, and only a very small amount of behavioral changes (e.g., stricter input validation); there is nothing that will affect how the examples behave AFAIK.

Do we need to add the button to the Usage examples too, or just these API reference examples would be enough for this PR?

I'd say API reference is a good first step here. I'm assuming that that is automatic, while Usage examples will require inserting directives into the .rst files - is that correct? If so, let's do API reference only to get some experience, and leave Usage examples for later.

agriyakhetarpal commented 3 months ago

I'd say API reference is a good first step here. I'm assuming that that is automatic, while Usage examples will require inserting directives into the .rst files - is that correct? If so, let's do API reference only to get some experience, and leave Usage examples for later.

Yes, the Usage examples will likely require using the NotebookLite directive. I just tested the display for all of the doctest-based examples, which are making use of the TryExamples directive (enabled by global_enable_try_examples = True in conf.py). All of them are working as expected on all but the following pages under API reference (where there are no doctest-based examples, but other code-block based examples are present):

  1. Other functions
  2. Overview of multilevel wavelet decompositions
  3. Signal extension modes
  4. Multiresolution Analysis

These can be updated in a separate PR as necessary, but I am happy to try configuring those examples to the doctests here too.

One thing that isn't working so far and what I'm currently investigating is the size of the JupyterLite notebook that gets loaded, for example:

This screenshot displays the PyWavelets documentation deployed with Sphinx locally, where a page from the API reference is currently open.

is too short and doesn't expand to display all of the code cells, in comparison to the deployment showcased in https://github.com/scipy/scipy/pull/20019. It might get fixed with a try-examples.json file that I am missing.

agriyakhetarpal commented 3 months ago

It might be fixed with a try-examples.json file that I am missing.

This was indeed the missing thing, now everything works!

rgommers commented 3 months ago

These can be updated in a separate PR as necessary, but I am happy to try configuring those examples to the doctests here too.

Sure, if you know how to do it then why not do it straight away. All I was trying to say is that incremental improvements are okay too.

This is starting to look pretty good!

agriyakhetarpal commented 3 months ago

Okay, I have now restructured:

Some more points:

  1. The Overview of multilevel wavelet decompositions page has its its reference files in doc/pyplots/ and they are end-to-end examples of analyses. It would be better to club changes to them in another PR, and so is the case for the Signal extension modes page; it contains a plotting example and some in-line examples, which I have marked with the .. try_examples:: directive manually like those mentioned in point 2 (side note: it is good thing PyWavelets isn't doesn't have a massive documentation reference haha).
  2. Also, Multiresolution Analysis, a.k.a. pywt.mra does not have any examples in the docstrings (just the two scripts in the demo/ folder).
  3. For the CWT examples, there are some that contain the # doctest: +SKIP label (but only inside the JupyterLite notebook, not the Sphinx docs). Is there something we can do about them – we don't want to break the doctests either? I do not think it is a big deal, though.

This is ready for another review, whenever you get a chance to do so! The aftermath of this PR can take care of the other pages. Maybe we should use nbsphinx or Myst-NB to render them as actual notebooks when they have been converted to such (the latter would be better and more stable in its configuration – it's developed more actively).


P.S. I modified the CSS to make the buttons left-aligned instead of being right-aligned (like SciPy's were). I feel that the left-aligned buttons look better under a left-aligned heading, but that's just my preference. Do you have thoughts on that?

agriyakhetarpal commented 3 months ago

Okay, this is strange. I am receiving errors like File Load Error for 158988af_6816_4f65_a71c_affe1c6d5d90.ipynb when I try to start the kernel on any example on Read the Docs, but it works perfectly locally. The button is right-aligned too, and not left-aligned as I had configured it.

Edit: seems to be resolved now!

agriyakhetarpal commented 3 months ago

Another thing I am noticing right now: the "Try it in your browser" text works on code snippets in docstrings under the

Examples
--------

heading, but it does not propagate to custom in-line examples, i.e., where we are using the .. try_examples:: directive manually – the button still shows the default "Try it with Jupyterlite" text instead of getting it from conf.py. jupyterlite-sphinx does offer a :button_text option to configure a particular example's button's text, but if it isn't present – it should consider the global configuration value.

This is as observed on the Continuous Wavelet Transform (CWT) page, and I think this is a bug. Let me file this on jupyterlite-sphinx's issue tracker.

agriyakhetarpal commented 3 months ago

Actually, I was working on submitting a PR today for the issue I opened over at jupyterlite-sphinx and tagged above – I should have asked you here to hold off on merging this :) But that's not a big deal and it is a minor fix too. I hope to be done with that soon, and I can always put up another PR.

rgommers commented 3 months ago

That's perfectly okay I think - I'm sure there will be several follow-up PRs to this one to fix some issues and polish the experience.