Skip to content

Migrate from nbsphinx/jupyter-sphinx to myst_nb#11456

Open
VeckoTheGecko wants to merge 65 commits into
pydata:mainfrom
VeckoTheGecko:myst-parser-support
Open

Migrate from nbsphinx/jupyter-sphinx to myst_nb#11456
VeckoTheGecko wants to merge 65 commits into
pydata:mainfrom
VeckoTheGecko:myst-parser-support

Conversation

@VeckoTheGecko

@VeckoTheGecko VeckoTheGecko commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Description

Here we migrate from jupyter-sphinx to myst_nb.

First I found all files with jupyter-sphinx directives, used rst2md on these files, and then went through manually fixing errors using sphinx-build -W (fail on warning) so that we have no more warnings.

TODO:

Separate issue/PR

  • Look into terminology.rst

    • is it possible to use the codeblocks run in CI here even though its nested?
    • if we can update this file, we can get rid of the jupyter-sphinx dependency
  • Update contributing guidelines

Regressions - general

  • Notebooks in the examples/* folder no longer have the nbsphinx prolog linking to the binder instance
image

Regressions - visual

Nested code cells not possible - code cells have been moved outside admonitions

Screenshot 2026-07-19 at 23 28 38 image

Was getting the following errors:

/Users/Hodgs004/coding/repos/xarray/doc/user-guide/plotting.md:392: WARNING: Found an unexpected code-cell or raw-cell directive. Either this file was not converted to a notebook, because Jupytext header content was missing, or the code-cell was not converted, because it is nested. See https://myst-nb.readthedocs.io/en/latest/authoring/text-notebooks.html for more information. [mystnb.nbcell]

indicating code cells in admonitions aren't possible.

See 12a18f9 for all items changed.

I think this error is from myst_nb and integration with Sphinx, and is not a problem with the myst language or tooling that uses native myst doc builders (looking at the Myst guide this cell in callout is possible).

Nested code cells not possible - tabbed sections with code blocks now just separate headings

(similar to above regression)

image

Checklist

  • towards Migrate from nbsphinx to myst, myst-nb #7924 (will close once we get rid of all rst?)
  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR. Tools: Claude - very minimal

@VeckoTheGecko
VeckoTheGecko marked this pull request as draft July 19, 2026 17:48
Was getting the following errors:

/Users/Hodgs004/coding/repos/xarray/doc/user-guide/plotting.md:392: WARNING: Found an unexpected `code-cell` or `raw-cell` directive. Either this file was not converted to a notebook, because Jupytext header content was missing, or the `code-cell` was not converted, because it is nested. See https://myst-nb.readthedocs.io/en/latest/authoring/text-notebooks.html for more information. [mystnb.nbcell]
These cells were visually merged in prior docs.
@VeckoTheGecko
VeckoTheGecko marked this pull request as ready for review July 20, 2026 04:20
Comment thread doc/get-help/faq.md
Automatic interpretation of labels is powerful but also reduces flexibility.
With xarray, we draw a firm line between labels that the library understands
(`dims` and `coords`) and labels for users and user code (`attrs`). For
example, we do not automatically interpret and enforce units or [CF

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't know how long ago this was written but we do now automatically interpret CF conventions for the zarr backend. But maybe this means something else?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I mean, we understand CF time conventions but that's about it no? We don't go for the iris approach of understanding more cf conventions.

I agree though that maybe the wording here can be improved. For a future PR if you have ideas?

Comment thread doc/get-help/faq.md Outdated
Comment thread doc/getting-started-guide/quick-overview.md Outdated
Comment thread doc/getting-started-guide/quick-overview.md
Comment thread doc/getting-started-guide/quick-overview.md Outdated
Comment thread doc/user-guide/complex-numbers.md Outdated
@VeckoTheGecko

Copy link
Copy Markdown
Contributor Author

Thanks for the review @eni-awowale ! I have finished going through all the pages that were changed, fixing a handful of (some pre-existing) rendering issues.

The only regressions we have are documented in the PR description now.

Is there anything else that we should consider before merging this?

  • Check doc build time before and after

Checking the logs on RTD, it looks like the build time dropped from 11min to about 9.5min

@eni-awowale

Copy link
Copy Markdown
Collaborator

This is awesome 🦎!

I think the latest commit broke the docs build? I was able to get the docs for the PR before.

@VeckoTheGecko VeckoTheGecko added the plan to merge Final call for comments label Jul 22, 2026
@VeckoTheGecko

VeckoTheGecko commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the build, thanks!

Do you have any opinions on the regressions? I think we're good to go right? (though maybe we leave this open for a couple days in case maintainers have other comments)

@eni-awowale

eni-awowale commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

For the regressions with the warnings maybe it would be clearer to have the python code outside of the warning admonition? So the warning reads more like a sub-header for that section and it is more consistent with how it is done with the note admonition regression you showed.

I don't really have strong opinions either way, so i think it's good to go! But it might be nice to leave this open and see if other folks have any other ideas.

EDIT:
Idk if the error you mentioned can't be duplicated locally but when I add a code cell to the admonition it works and renders correctly?
Screenshot 2026-07-22 at 3 56 04 PM

@VeckoTheGecko

Copy link
Copy Markdown
Contributor Author

Idk if the error you mentioned can't be duplicated locally but when I add a code cell to the admonition it works and renders correctly?

Hmmm, how are you adding the code cell?

I was doing it via


````{note}

Some admonition body

```{code-cell}
print("some code")
```

````

And that wasn't working for me

@eni-awowale

Copy link
Copy Markdown
Collaborator

this is what i did:

````{warning}
Do not try to assign values when using any of the indexing methods `isel`
or `sel`:

```python
# DO NOT do this
da.isel(space=0) = 0
```

Instead, values can be assigned using dictionary-based indexing:

```python
da[dict(space=0)] = 0
```

Assigning values with the chained indexing using `.sel` or `.isel` fails silently.
See the cell below:

{code-cell}
da = xr.DataArray([0, 1, 2, 3], dims=["x"])
# DO NOT do this
da.isel(x=[0, 1, 2])[1] = -1

````

@VeckoTheGecko

Copy link
Copy Markdown
Contributor Author

That doesn't work for me @eni-awowale :/ . Could you try pixi run doc-clean?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

io plan to merge Final call for comments topic-backends topic-zarr Related to zarr storage library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants