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

Upgrade myst-parser dependency to 0.12.x #591

Closed
mwouts opened this issue Aug 20, 2020 · 6 comments
Closed

Upgrade myst-parser dependency to 0.12.x #591

mwouts opened this issue Aug 20, 2020 · 6 comments
Milestone

Comments

@mwouts
Copy link
Owner

mwouts commented Aug 20, 2020

See also #589 . Cc @chrisjsewell .

@mwouts mwouts added this to the 1.6.0 milestone Aug 20, 2020
@chrisjsewell
Copy link
Contributor

Heya, yeh at some point in the next week or 2, I want to do a 2nd "parse at the parser" here, taking into account our discussion in executablebooks/MyST-NB#214 and looking to remove the myst-parser dependancy (maybe just using the underlying markdown-it-py dependency)

@mwouts
Copy link
Owner Author

mwouts commented Aug 20, 2020

That sounds great! Although I don't want to cause you lots of work. Personally I was thinking of just making Jupytext 1.6.0 depend on myst-parser==0.12.x, but you're right, maybe this is an opportunity to just take a dependency on markdown-it-py. Take your time to think about it, you will tell me which option you prefer.

FYI I recently merged in master a commit that ensures that MyST files are recognized as such even when myst-parser is not installed (#584), but I still have some work to do on 1.6.0 (two weeks or more on my side as well 😄 ) before I can release it...

@chrisjsewell
Copy link
Contributor

I was thinking of just making Jupytext 1.6.0 depend on myst-parser==0.12.x

Yeh the only issue with that, is that it now has a direct dependency on sphinx (rather than in an extra), which would make jupytext quite bloated.
Whereas, as I mentioned before, markdown-it-py only depends on attrs.

FYI I recently merged in master a commit that ensures that MyST files are recognized as such even when myst-parser is not installed

That's great thanks 😄

@chrisjsewell
Copy link
Contributor

FYI, MyST-NB (and thus jupyter-book) now supports using any jupytext conversion 😄 (see https://myst-nb.readthedocs.io/en/latest/examples/custom-formats.html)

When writing this page,
I came across two issues:

  1. The myst-parser version incompatibility that I want to obviously fix here
  2. As you can see I wanted to show the block:
````md
```{python echo=TRUE}
import pandas as pd
series = pd.Series({'A':1, 'B':3, 'C':2})
pd.DataFrame({"Columne A": series})
```
````

However, this ended up messing up the jupytext conversion, with the code-chunk after it no longer being identified as a code cell.
It appears the .Rmd conversion does not support having variable ` lengths for code fences?

If so, this leads me on to another suggestion; that if markdown-it-py is going to be a dependency of jupytext, then it can be used to do some of the required Markdown parsing, rather than having more "bespoke" parsing functions.
This is what I already do in:

def myst_to_notebook(

Could take a bit of work though, so I wouldn't expect this any time soon 😉

@mwouts
Copy link
Owner Author

mwouts commented Aug 25, 2020

Hello Chris,

It appears the .Rmd conversion does not support having variable ` lengths for code fences?

Yes, that's right, the parsing system is Jupytext is not very elaborate. When I wrote it I was not aware of the possibility of using more than three backticks... Of course we could consider switching to markdown-it-py, but as you mention there is some work involved here! Meanwhile, for your use case of quoting a code cell, maybe you could indent the code cell, can't you?

Regarding the version of myst-parser than I should make Jupytext depend on, for the next release, do you prefer that

  1. We make jupytext 1.6.0 depend on myst-parser 0.11.x (no dev involved)
  2. We make jupytext 1.6.0 depend on myst-parser 0.12.x and fix the issue documented at Keyword argument 'disable_syntax' is not a supported parameter name of function default_parser. #589?

@chrisjsewell
Copy link
Contributor

chrisjsewell commented Aug 25, 2020

  1. We make jupytext 1.6.0 depend on markdown-it-py (fixing Keyword argument 'disable_syntax' is not a supported parameter name of function default_parser. #589), since the only part of myst-parser we need is just a thin wrapper around it anyway 😄

I will make a PR in the next day or 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants