executablebooks / MyST-NB

Parse and execute ipynb files in Sphinx
https://myst-nb.readthedocs.io
BSD 3-Clause "New" or "Revised" License
206 stars 82 forks source link

Change the metadata we use to check for "is a myst markdown notebook" #214

Open choldgraf opened 4 years ago

choldgraf commented 4 years ago

Currently we check for jupytext metadata in order to decide if a markdown file is a "myst markdown notebook":

https://github.com/executablebooks/MyST-NB/blob/9f889241f40336ab138391176486166b0881715d/myst_nb/converter.py#L65

I think that this is sub-optimal for two reasons:

  1. It effectively embeds Jupytext metadata as part of the MyST notebook spec (since the Sphinx extension is the reference implementation of the MyST notebooks spec and this is required for MyST-NB to use them)
  2. It requires users to put stuff that isn't strictly-related to running their pages in the page metadata.

I wonder if we could instead change the definition of a "MyST Markdown Notebook" to be either:

  1. Something that has myst-notebook: true in the header.
  2. Something that has a kernelspec: section in the header.

Then we could still use jupytext to convert to ipynb at build time, but we don't need to use the full Jupytext metadata to trigger it.

What do folks think?

chrisjsewell commented 4 years ago

How would you still use jupytext, without jupytext metadata? Also this would make these files incompatible with binder/jupyterhub.

It may be possible to change the metadata that jupytext uses to identify myst notebooks, but obviously this must be first done upstream in that repo

choldgraf commented 4 years ago

Well I believe that jupytext can be run on any file if you call the input / output explicitly.

E.g. in a markdown file like:

# Title

```{code-cell}
print('hi')

and then run `jupytext myfile.md --to ipynb --from myst`

then it works fine. 

If you then add metadata about the kernel like

kernelspec: display_name: Python 3 language: python name: python3

Title

print('hi')


Then it also adds the kernel metadata to the resulting ipynb file.

good point about making it incompatible with binder/jupyterhub - though that's something we can always document "if you want auto-opening in jupyterhub, you need to add jupytext metadata".

tbh I feel like the minimal thing that should be needed is just a kernel name in the metadata, rather than the full kernelspec - that's something that I think most people can remember rather than requiring them to look it up or use a helper function like `jupyter-book myst`
chrisjsewell commented 4 years ago

Only using kernelspec would not distinguish it from other jupytext markdown formats though. I would definitely look at what is possible in jupytext first, for example orphan and tocdepth is already set to be preserved in round-trip operations by default:

https://github.com/mwouts/jupytext/blob/b3655ece8662cefc9ab9c314e8f83b74ce297214/jupytext/metadata_filter.py#L14-L16

So there may be scope to also add myst-notebook to this list

Note I already want to "refresh" how myst markdown is parsed in jupytext: https://github.com/mwouts/jupytext/issues/556#issuecomment-656264281, it would be ideal if the approach in this repo was similar to that

chrisjsewell commented 4 years ago

I guess the main confusion currently faced by users of myst markdown with jupytext, is that jupytext defaults to using the jupytext markdown format if there are any issues with the myst metadata with not much warning

choldgraf commented 4 years ago

yeah - mostly I am just trying to make the "minimal number of steps to making a myst markdown notebook" as short as possible. The ideal would be for people to just open a text editor and start writing MyST Notebook Markdown. A slightly more cumbersome approach is to require myst-notebook: true or somesuch thing in the header. I think right now we make this a bit too cumbersome, because the user will have to go back to a terminal and run a CLI command then return to their editor window to see the newly-added metadata

mwouts commented 4 years ago

Hi @chrisjsewell , @choldgraf , well that is an interesting question!

I came to the same issue myself when I wanted to write a test test_meaningfull_error_open_myst_missing...

I think I have a fix for the jupytext part of the question at https://github.com/mwouts/jupytext/pull/584/commits/535766feb5f4f3056acb0a95f76d0c84e5275c7c . With this, Jupytext will correctly recognize a md:myst file even if myst is not installed.

The approach that I have used there is the following: does

metadata.get("jupytext", {})
                .get("text_representation", {})
                .get("format_name")
                == "myst"

We can of course change that. Maybe indeed we can distinguish between MyST-Markdown and Jupytext-Markdown documents based on whether a jupytext or kernelspec key is found at the root level of the YAML format.