Changes for multi-version docs

[animeshsasan]

Description

What is this PR

• Addition of a new feature

Why is this PR needed?
This PR is the implementation for neuroinformatics-unit/actions#91.
At present, our build_sphinx_docs and deploy_sphinx_docs actions only deploy a single version of the documentation (either from main or the latest release) to GitHub Pages. In brief:

build_sphinx_docs builds the Sphinx docs and uploads an artefact
deploy_sphinx_docs downloads the artefact and copies it to the root of the gh-pages branch
This means users and contributors cannot view older versions of the docs, except by checking out an old commit and building locally. Having access to documentation for previous releases would be extremely useful.

What does this PR do?
The PR adds the version switcher to the docs along with changing the document deployment step to the newly created multiversion deploy step.

References

A similar implementation is used in the NeuroBlueprint repository here.
Please reference any existing issues/PRs that relate to this PR.

How has this PR been tested?

A test implementation exists by forking this repository and making changes to enable the multi-version switcher. You can see it in action here.

Is this a breaking change?

The document deploy step is changing but no actual features are changed.

Does this PR require an update to the documentation?

It requires updating the movement repository to create the multi-version dropdown but no actual documentation has changed.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality(I don't think there are any tests for the docs)
• The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a51b11d) to head (53c1622).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #668   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           34        34           
  Lines         1981      1981           
=========================================
  Hits          1981      1981           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[niksirbi]

Thanks @animeshsasan.

Please read the following in the context of my review of neuroinformatics-unit/actions#93.

Because of the mentioned change in URLs, some links to specific documentation pages need to be updated in this repo's README, e.g. links to installation instructions and examples. You won'd need to update the main website link itself, i.e. movement.neuroinformatics.dev — if you take my advice about a redirect. You also need not worry about internal links in the website, as those rely on sphinx cross-references instead of URLs, and should be robust to the change.

The slightly bigger issue is with binder, which we use in combination with sphinx-gallery to allow our examples to run on binder instances on the web. See docs on sphinx-gallery + binder configuration. If you look higher in the conf.py file you'll see that binder pulls the examples from the gh-pages branch, which currently has an "examples" folder in the root. Now that folder will be under "latest/examples", so I'm worried that the binder integration may break. I'm not sure how to test this exactly, unless we also add some sphinx-gallery examples on the fork. Could you investigate this and see whether we'll have problem with that?

Note that NeuroBlueprint doesn't include sphnx-gallery examples, so that was never an issue.

[animeshsasan]

I’ve updated the links in the README to include the /latest part of the URL.

To verify how the documentation build would look, I built and deployed the docs on my fork here.

From what I found:
• The /examples directory now sits under /latest.
• A /binder folder is also created under /latest.

So it seems this restructuring should not cause any issues. You can view the deployed documentation here: example page.

One thing I did notice while testing: the Binder link fails to launch for the docs deployed from the main repo as well. I might be missing something, but if Binder is indeed broken there, we may need to fix that separately.

[animeshsasan]

I also wanted to check whether we should clean up the existing gh-pages branch before merging this PR. Technically it shouldn’t cause any issues since the current changes overwrite the index.html to redirect to /latest, but I thought I’d confirm with you first.

[niksirbi]

One thing I did notice while testing: the Binder link fails to launch for the docs deployed from the main repo as well. I might be missing something, but if Binder is indeed broken there, we may need to fix that separately.

That was working fine a few days ago (we tested it after the latest release and nothing changed since then). According to the error message, the problem is with BinderHub itself which is too busy at the moment. Similar issues have cropped up in the past. That said, I was able to make it work after a few tries, but we probably have to wait a bit for BinderHub to quiet down.

So it seems this restructuring should not cause any issues. You can view the deployed documentation here: example page.

I was actually able to run the examples from your fork on Binder! Thanks for making a fork and deploying a website from there, it was super useful to verify functionality.

[niksirbi]

I also wanted to check whether we should clean up the existing gh-pages branch before merging this PR. Technically it shouldn’t cause any issues since the current changes overwrite the index.html to redirect to /latest, but I thought I’d confirm with you first.

I'm a bit wary of doing this right away, as it will potentially take down our website. My current tentative plan is do the following:

• Migrate NeuroBlueprint to use the action we've just merged (I can open a PR for that), and make a new patch release there to see if everything goes smoothly in deployment.
• Then merge this PR here and see what happens. Technically is should create a "dev" version and add the switcher, right? I want to set aside some time for that to be able to roll back if necessary. I can combine it with the next release of movement. I can clean up the older contents of gh-pages after the release.

What do you think?

[niksirbi]

Testing on NeuroBlueprint first turned out to be a good idea. We discovered some hiccups in the process:

1. The displayed release number at the top left for the "dev" version would be misleading in some cases. This is due to how we were parsing the release number in NeuroBlueprint's conf.py file. See the fix here: Changes for multi-version docs #668. This won't be a problem for movement because we are already parsing the release number appropriately.
2. For "dev" version deployments the action was not actually fetching the switcher.json from latest, due to how the conditional logic was structured within the action. Together with @IgorTatarnikov we've fixed it, see Fetch switcher.json content from latest for dev builds actions#103 and Add missing dollar sign in deploy_sphinx_docs_multiversion action actions#104.

Apart from the above issues, which are now patched, there is one remaining snug: if you look at the version switcher dropdown at https://neuroblueprint.neuroinformatics.dev/latest/index.html, it displays "Choose version" no matter which version we've selected (I belatedly realised this is also the case on your forked website test). The expected behaviour is for the currently selected version to be displayed there, as is the case for the NumPy docs website: https://numpy.org/doc/1.26/index.html

Could you look into it?
After we find a solution to this snug, I will be happy to switch movement to the new action.

[animeshsasan]

Hey @niksirbi,
Thanks a lot for reviewing the PR and testing the changes! I really appreciate the thorough feedback.

Regarding the second issue, I want to clarify my understanding: in the new changes, we’re updating the switcher.json in the dev build to match the one in latest. Since the live website always fetches the switcher.json from /latest, I wasn’t clear why the contents of the dev file itself would matter. Even when we sync the content, we’re just copying from /latest, so an older switcher.json in dev shouldn’t have an effect, unless there’s a subtle detail I’m missing. Could you clarify that part?

As for the problem with the version not being displayed in the switcher dropdown: I’ve tracked this down. The version shown in the switcher is determined by the value of html_theme_options["switcher"]["version_match"]. This value is compared against the version fields in the switcher.json, and if they match, the corresponding version label is displayed.

The bug is that our version_match was just <version_number>, while the switcher.json entries are of the form v<version_number>. Because of that mismatch, the theme couldn’t find a match and defaulted to showing “Choose version”. I’ve updated the PR with the fix and tested it here.

I also suspect something similar has been happening in the NeuroBlueprint docs. For example, v0.3.0 shows up correctly in the switcher when selected, but newer versions do not. That suggests there may have been a change in how version_match was being set at some point.

One more detail: if version_match is supplied with an incorrect value, the switcher will fall back to whatever value is supplied in version_match. For instance, in my test docs the logo text (which uses the same variable) shows v0.1.14 even when browsing v0.1.15 or v0.1.16. The switcher itself will still let you navigate, but the displayed label comes directly from the mismatched version_match.

[IgorTatarnikov]

Re neuroinformatics-unit/actions#103, your interpretation is correct in that the dropdown always uses the ../latest/_static/switcher.json to fetch data for that element. This ended up being a red herring for the dropdown text bug. I should have refreshed myself on the docs prior to debugging!

[niksirbi]

It was indeed a red herring, but I'd stick with the current form of the action anyway. It makes more conceptual sense for the switcher.json in the dev folder to be up-to-date with the one in latest, even if it's not actually being used. No harm done either way.

The bug is that our version_match was just <version_number>, while the switcher.json entries are of the form v<version_number>. Because of that mismatch, the theme couldn’t find a match and defaulted to showing “Choose version”. I’ve updated the PR with the fix and tested it here.

That seems correct to me and would be an easy fix. But if you are right we'd still have the problem for the dev branch, right? The label in the json file is dev, but the release variable would be sth like 0.3.4.dev5, leading to a mismatch (prepending the v wouldn't solve that case). In fact, on your fork funny things happen when switching to the "Development" version, which may be caused by the mismatch I described. Perhaps we need to add some custom logic to the conf.py for transforming the release variable into something suitable for the switcher across all version cases, including dev?

[animeshsasan]

Makes sense! I agree it’s still useful to keep the changes and having the switcher.json consistent between dev and latest helps with clarity, and there’s no downside.

For the version-matching issue, I think a small transformation in conf.py should cover the edge cases. Something like:

try:
    release = setuptools_scm.get_version(root="../..", relative_to=__file__)
    release = release.split("+")[0]  # remove git hash
except LookupError:
    # if git is not initialised, still allow local build
    # with a dummy version
    release = "0.0.0"

if "dev" in release:
    doc_version = "dev"
else:
    doc_version = f"v{release_clean}"

I am new to setuptools_scm, but from what I understand about how it works, the cases break down like this:

• On an exact tag (e.g. v0.3.4) → release = "0.3.4"
• On a commit past a tag (e.g. 5 commits past v0.3.4) → release = "0.3.4.dev5+g123abc"
• With local modifications → something like 0.3.4.dev5+g123abc.dirty (don't need to consider for our use case)

The if check should normalize all of these into either "dev" or "vX.Y.Z", which then lines up with the entries in switcher.json. In the rare case of a LookupError, Sphinx would fall back to showing Choose version, which seems acceptable.

Do you think there are other setuptools_scm version formats or special cases we’d need to handle, or would this be sufficient?

Some context from my fork:
The way release is being set there is different. Instead of using setuptools_scm, it runs

result = subprocess.run(
        ["git", "describe", "--tags", "--abbrev=0"],
        capture_output=True,
        text=True,
        check=True,
    )

This command only returns the most recent tag (e.g. 0.1.16) and strips the leading v. That’s what’s causing the mismatch for both the Development build and for older tagged versions like v0.1.15 and v0.1.16(I just created a new tag and pushed in the later cases without a commit). This should not happen with setuptools_scm since it always returns the latest tag.

[niksirbi]

You may go ahead with implementing it as shown in your snippet @animeshsasan.
Your understanding of setuptools_scm matches mine, so I think this should cover all bases.

[niksirbi]

@animeshsasan I applied your suggested fix to NeuroBlueprint, and made a new v0.4.0 release to fully test the new action.
Something appears to have gone wrong in the editing of the latest switcher.json. This is what we got:

[
  {
    "version": "dev",
    "url": "https://neuroblueprint.neuroinformatics.dev/dev/"
  },
  {
    "name": "v0.4.0 (latest)",
    "version": "v0.4.0",
    "url": "https://neuroblueprint.neuroinformatics.dev/latest/latest"
  },
  {
    "version": "v0.3.3",
    "url": "https://neuroblueprint.neuroinformatics.dev/latest/v0.3.3"
  },
  {
    "version": "v0.3.2",
    "url": "https://neuroblueprint.neuroinformatics.dev/v0.3.2"
  },
  {
    "version": "v0.3.1",
    "url": "https://neuroblueprint.neuroinformatics.dev/v0.3.1"
  },
  {
    "version": "v0.3.0",
    "url": "https://neuroblueprint.neuroinformatics.dev/v0.3.0"
  }
]

As you can see the URLs for both the latest (v0.4.0) and the previously latest (v0.3.3) releases are wrong:

• The former now ends up with latest/latest
• The latter has an extra /latest/ which should have been removed.

[niksirbi]

The problem actually comes from this line of the action

BASE_URL="$(jq -r '.[1].url|split("/")[0:-1]|join("/")' <<< $SWITCHER_CONTENT)"

which is supposed to remove the last element of the URL (after the last slash). This is not robust to trailing slashes, and one such trailing slash was present in the URL of version v0.3.3.

This edge case arose specifically in NeurBlueprint, because our previous hacky workflow was appending /latest/ to the new URL. I don't think we would have encountered this in movement, but in any case we should harden the implementation against such slash mishaps.

An easy fix would be to strip any trailing slashes first, then remove the /latest (sth like below, untested), but open to alternative suggestions as well.

# Derive a stable BASE_URL even if the switcher has trailing slashes
BASE_URL="$(jq -r '.[1].url
| sub("/+$"; "")     # drop trailing slash(es)
| sub("/latest$"; "")' <<< "$SWITCHER_CONTENT")"

NEW_URL="${BASE_URL}/latest"

[niksirbi]

Note that for the NeuroBlueprint case, I've patched things up by directly pushing the fix to the gh-pages branch (bad practice, I know).

This completely fixed the dev and v0.4.0 (latest) versions, but some of the older versions still have that "Choose version" bug in the switcher. I suspect that's because conf.py was not correctly matching the version name when the docs for those versions where built. We've discussed with @JoeZiminski and decided to just live with that for the NeuroBlueprint case (I think this was anyway the status quo before we applied the new action).

None of this should be a problem for future versions of NeuroBlueprint, or any other package like movement that adopts multi-version docs for the first time. We just have ensure that:

• our json querying is robust to trailing slashes, as mentioned above
• we document the doc_version pattern for conf.py in the action's README

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[animeshsasan]

I’ve updated the current PR to include the if condition you suggested, replacing the release variable with doc_version for the version_matcher.

Great catch, @niksirbi ! You’re right, this likely wouldn’t cause problems for repos adopting multi-version docs for the first time through our action, but as you pointed out, projects like NeuroBlueprint that already have a switcher.json in place can definitely run into this edge case.

To address this, I’ve opened a PR with the proposed fix here: neuroinformatics-unit/actions#105. Thanks again for flagging this!

[niksirbi]

I'm approving this PR as good-to-go now, thanks @animeshsasan 🎉 .

I'll merge it shortly before the next release and set aside some time to deal with anything we might have missed (though I'm now reasonably confident that we've caught all edge cases).