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

Use sphinx gallery machinery to directly build how-to file #3472

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JoeZiminski
Copy link
Collaborator

@JoeZiminski JoeZiminski commented Oct 11, 2024

This PR introduces a new way of building the how-to files, further to #2879 (discussed in #2881 and more recently #3191). It uses the short "Combine recordings in SpikeInterface" page as proof of principle. This is a simple example (the original was just a .rst file) but can be extended to ther existing .py files that require manually building.

Briefly, at present, how-to files are either provided directly as .rst files or manually built (as described here). The downsides of this approach are:

  • It is manual (i.e., long and boring) to build.
  • It is easy to break by changing code and forgetting to update documentation (e.g. with the .rst file as in this example).

Building documents with sphinx-gallery fixes these issues by a) automating the process and b) raising warnings/errors if the .py files error out due to changes in the codebase. One issue previously discussed in #3191 is what to do about pages that take a long time to build. For these cases, we can extend the approach here by naming these files something like long_build_plot_<my_doc>.py with a custom tag, e.g., --long-build, that extends the plot_ prefix for Sphinx full-building to long_build_plot when these must be fully built.

This PR:

  • (for now) introduces a new 'how_to_new' folder in spikeinterface/examples that holds the .py files for Sphinx to build. This is built by Sphinx and output to docs/how_to_new, and the index.rst in docs/how_to now points to this.
  • docs/how_to_new (which holds the Sphinx-built files) is added to .gitignore.

It would be great to hear what people think of this. If people approve, a tentative plan for the next steps is:

  1. Quickly move all short how-to pages to this format.
  2. Move the long-time-to-build pages (e.g., drift correction) to this format. In some cases, we will convert real-world data examples into synthetic examples. In cases where it will be useful to maintain the real-world examples (e.g., drift correction), we can create a new section (e.g., 'Real World Examples') where it is made clear that the code is exemplary and not guaranteed to run or meant to be a code-along tutorial.
  3. After this, we can make the current 'Tutorials' page a custom index.rst rather than built by Sphinx Gallery. This way, we can have full control over what we want to include in how-tos versus tutorials.

TODO:

  • start with converting standalone .rst files (where relevant)
  • try to build with github CI
  • see how long it takes to build long-build docs (can we get under RTD 10m?)

@chrishalcrow
Copy link
Collaborator

This looks great!

For the 'Real World Examples', is the idea that these are made locally, and plots etc are just uploaded as images by the creator? If so, this sounds a bit like our current set-up for the long notebooks (quickstart, drift and something else). I saw that the nbsphinx extension allows someone to upload a already-executed notebook (https://docs.readthedocs.io/en/stable/guides/jupyter.html + https://nbsphinx.readthedocs.io/en/0.5.0/pre-executed.html). This might be a simpler solution for these real-data notebooks, compared to the existing: examples/*.py files which get turned into /doc/*.rst files.

I also saw that MNE have author credits for some of their tutorials (https://mne.tools/stable/auto_tutorials/time-freq/20_sensors_time_frequency.html#sphx-glr-auto-tutorials-time-freq-20-sensors-time-frequency-py). This might be good for the real-world examples, as it gives users a contact point if they have problems with these more-complex tutorials. This might also emphasise that these are a bit different, and not so carefully maintained by the core team?

@zm711
Copy link
Collaborator

zm711 commented Oct 14, 2024

The tutorial/how_to you choose was never meant to actually be built or run. Heberto and I made that specifically because Intan is a commonly used recording software and it has the issue that its default is to make a new recording every one minute. The new how to as written defeats the purpose of the original how to.

The way Heberto and I were thinking about this in general was:

We have a specific problem that can be generalized for users so let's solve the specific problem for users by highlighting the general machinery.

the new way removes part of the "specific problem + specific solution" and only keeps the general concept with the specific problem mentioned in the text. I'm good for thinking about outbuilding docs, but maybe we should think about the underlying purpose of the writing of some of the how to's. The idea is that this how to would never need to be written because it is a specific issue that has come up on the issues many times. It has one (or technically two) solutions which have been stable for a long time in the api.

@h-mayorquin opinions here? My vote is not editing this particular how to and maybe we find a way to tag how to/tutorial that are "frozen"/"solved" vs those that update with the fluid parts of the API (e.g. drift correction, neuropixel style data).

@zm711
Copy link
Collaborator

zm711 commented Oct 14, 2024

Sorry for the stream of consciousness. :)

@JoeZiminski
Copy link
Collaborator Author

Thanks @zm711, it didn't read like a stream of consciousness! I wish my streams of consciousness were as organised 😅

  1. For sure this way of building docs does not have to be used for every doc (I chose this one as it was relatively simple to edit). So if it's better to keep this as an .rst we can use it in the old way, and support a mix of the sphinx-built and raw .rst methods (the main aim is to avoid the .py -> .rst manual builds).

For this PR we can use this as an example and if people agree on the machinery, I can apply it to another example. However, I do think it would be better to reorganise this page, but we can discuss in a separate PR / issue (see thoughts below 😄 ).

Thoughts on this How To
2) I would suggest reorganising this page slightly in the interest of each documentation page have a single, very clear goal. At the moment, it is not clear if the goal is 'teach users how to combine recordings' or 'teach users how to load intan recordings'. Mixing these I think would a) confuse / distract people interested in how to combine recordings but who don't know what an Intan recording is b) mean people interested in loading Intan recordings miss this page, as they may not realise that combining recordings is the way to load Intan recordings.

So I think a very general code-along 'how to combine recordings' that works equally well for all, and then a separate 'Loading Intan files' tutorial (that can reference the 'how to combine recordings') would imo work better.

It also means that 'How to combine recordings' is not tested by sphinx auto-building. Although it's quite a stable part of the API, as it's so core I think it should still be tested, it only ever takes one PR to break something (and there is nothing worse than wrong documentation). If an Intan-specific tutorial breaks, it's less important as a) smaller target audience b) at least the general steps will be described and they can basically follow the (definitely working) linked 'how to combine...' page.

@zm711 zm711 added the documentation Improvements or additions to documentation label Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants