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

*always* use jupytext to read in markdown files? #88

Closed
choldgraf opened this issue Mar 19, 2020 · 5 comments · Fixed by #116
Closed

*always* use jupytext to read in markdown files? #88

choldgraf opened this issue Mar 19, 2020 · 5 comments · Fixed by #116
Labels
enhancement New feature or request

Comments

@choldgraf
Copy link
Member

I just wanna check my intuition here. Now that jupytext can read myst-markdown, why don't we simply always use jupytext to load in any markdown or ipynb file? If the file is not a "myst-notebook" style, then it'll simply be read in as a single markdown cell in a notebook, and then the parser should just treat it exactly as it would treat a regular .md file. If the file does have myst notebook syntax, then it will properly be read as a notebook. Any reason not to take this approach?

@chrisjsewell
Copy link
Member

chrisjsewell commented Mar 19, 2020

When we move to utilising jupytext, we will certainly be assessing/categorizing every .md file on whether it is a notebook or not.
The subtle question is do; you allow for the default jupytext format, or do you restrict to only the myst format, e.g. if you did jupytext.reads on the following, assuming that it wasn't specifically specified as myst in the metadata, it would convert to a notebook with a code cell:

```python
print("hi")
```

whereas if you now add a myst type code cell it would treat it as 1 markdown cell and 1 code cell:

```python
print("hi")
```
```{code-cell} python
print("there")
```

The other difference with using jupytext.reads as opposed to the "lower-level" functions in jupytext.myst, is that in jupytext.myst.myst_to_notebook I have added the ability to capture the source text line numbers, see: https://github.com/mwouts/jupytext/blob/a0676d71afed4bae4cd2fbfd108b44189e940fe8/jupytext/myst.py#L154

This is not available for any other format, and can't be utilised (currently) directly from jupytext.reads, unless if you could make a PR that somehow bubbled down the option to this function.

@choldgraf
Copy link
Member Author

ah that's a good point - propagating line numbers is a good reason to use our own function, I think. Though I guess that jupytext.reads is using myst_to_notebook under the hood anyway?

In that case, perhaps we should start by only using the myst parser, and using the myst_to_notebook function if a notebook is detected (which I guess would be done with a quick check for any {code-cell} lines)? Then in the future we can add jupytext support for other kinds of files

@chrisjsewell
Copy link
Member

Yes, this is where matches_msytnb comes in: https://github.com/mwouts/jupytext/blob/a0676d71afed4bae4cd2fbfd108b44189e940fe8/jupytext/myst.py#L38.

Another subtle point for consideration, is if the user end up having both notebook.ipynb and notebook.md in the sphinx source folder. Currently, I think sphinx just arbitrarily picks the extension relating to the first parser to be loaded. I raised an issue about it in sphinx-doc/sphinx#7324

@choldgraf
Copy link
Member Author

I think this would also require us to write our own parser for markdown files, right? If we wanted people to be able to write myst-nb markdown in .md files, we need myst-nb to be the parser that handles those files, not myst-parser (even though we may pass parsing duties to the myst parser if it's not a myst-nb file)

@chrisjsewell
Copy link
Member

i.e. #83

@chrisjsewell chrisjsewell linked a pull request Apr 1, 2020 that will close this issue
@chrisjsewell chrisjsewell added the enhancement New feature or request label Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants