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

Limited rst to md support in notebooks #219

Closed
chsasank opened this issue Apr 6, 2017 · 15 comments · Fixed by #705
Closed

Limited rst to md support in notebooks #219

chsasank opened this issue Apr 6, 2017 · 15 comments · Fixed by #705

Comments

@chsasank
Copy link

chsasank commented Apr 6, 2017

Hi,

Current parser for rst to md in notebook.py is quite limited. For example this page's notebooks looks like this:
screen shot 2017-04-06 at 12 13 02 pm

Here's what I propose: use pypandoc if available else fallback to current implementation. If you like this, I can raise a PR.

@lesteve
Copy link
Member

lesteve commented Apr 7, 2017

Great to see that PyTorch is using sphinx-gallery! To be fair this is probably one of the more involved sphinx-gallery "example as tutorial" I have seen in the wild (~600 lines).

My first instinct is to be slightly reluctant to go in this direction but I could be convinced if the PR has an optional dependency on pypandoc and is not too intrusive.

The first thing to check would be that pandoc does convert the rst to something reasonable in your example.

@chsasank
Copy link
Author

chsasank commented Apr 7, 2017

"example as tutorial"

Well, I moved pytorch tutorials from github hosted notebook files to html pages based on sphinx gallery. I think sphinx gallery is excellent for writing tutorials. I have a simple script here to convert notebooks to notebook styled examples based on pandoc.

that pandoc does convert the rst to something reasonable

It works great.
screen shot 2017-04-07 at 10 36 51 pm

not too intrusive.

Here's what I want to do:
edit this function to this:

def rst2md(text):
    """Converts the RST text from the examples docstrigs and comments
    into markdown text for the Jupyter notebooks"""
    try:
        import pypandoc as pdoc
        return pdoc.convert_text(text, 'md', 'rst')
    except ImportError
        <Current code>
        ... 

That's non intrusive enough, I hope?

@chsasank chsasank closed this as completed Apr 7, 2017
@chsasank chsasank reopened this Apr 7, 2017
@lesteve
Copy link
Member

lesteve commented Apr 7, 2017

The change does not look too intrusive indeed. Maybe others (@Titan-C @GaelVaroquaux @Eric89GXL) have comments on this before you start working on a PR.

A small comment: note that when downloading the notebook you probably won't have neuralstyle.png lying around so that the image will be missing in the notebook.

@larsoner
Copy link
Contributor

larsoner commented Apr 7, 2017

If the proposal is to (if possible) offload the conversion onto a better-tested library, then +1 from me

@Titan-C
Copy link
Member

Titan-C commented Apr 8, 2017

The last time someone suggested using pandoc the discussion immediately ended with NO. because that includes putting the entire GHC and pandoc as a dependency. At those times we were not aware pypandoc existed nor that pandoc now can be installed as a binary.

I just tested pypandoc and it is capable to download and install pandoc in userspace, is also in conda-forge and windows wheels ship the pandoc binary. So installing this dependency becomes very easy now.
I agree on handing over as optional dependency the rst to markdown conversion to a specialized library. The try-except block is unintrusive enough for me and is the same way we support mayavi.

+1

@ambrosejcarr
Copy link

We're trying to using sphinx-gallery for our package, but running into problems where notebooks generated by sphinx-gallery don't convert properly, which has us reverting to checking in notebooks for the time being.

Came here to +1 the issue.

@romanlutz
Copy link

As far as I can tell this problem still exists, i.e. downloading notebooks from sphinx-gallery means they have rest-formatted markdown which doesn't render properly (like in the screenshot in the first post). Does anyone have workarounds?

sphinx-gallery is such a great effort, would be a shame not to be able to use it because of this.

@lucyleeow
Copy link
Contributor

@larsoner happy to implement the above proposed (optional) pypandoc solution if we're still +1

@larsoner
Copy link
Contributor

Yep, let's go for it with a try/except so that existing users don't have to migrate but can if they want, and hopefully get nice features/better outputs for having done so

@choldgraf
Copy link
Contributor

I'm +1 on this but we should flag it as a big old "this is advanced and your experience may vary" because pandoc can be pretty tricky to deal with different edge-cases etc, and I don't think we want to be on the hook for explaining how it works to folks.

Here's another thought: as a part of another project I've been working on, we've now got a 100% functional markdown parser for Sphinx (https://myst-parser.readthedocs.io/). This goes above what recommonmark does, and adds syntax for roles, directives, etc. Perhaps it could another optional extension / flag in sphinx-gallery, and then people could write their python comments in markdown, and the generated .ipynb would be in markdown as well (it wouldn't be CommonMark markdown, but MyST markdown is a superset of it)

@GaelVaroquaux
Copy link
Contributor

GaelVaroquaux commented Jun 10, 2020 via email

@choldgraf
Copy link
Contributor

@GaelVaroquaux I don't think this would be too difficult in principle, the myst-parser "just works" in the sense that if you add it as an extension then files ending with .md will now be parsed into the Sphinx doctree.

I think the question is how much there are assumptions made about rST in the sphinx-gallery code...that may be a big trickier.

Also I should probably clarify the above - if the user writes their content in CommonMark, then the resulting ipynb file would also be totally CommonMark-compliant. The only reason it wouldn't be CommonMark compliant is if, for example, they used roles and directives in their markup.

@larsoner
Copy link
Contributor

I think the question is how much there are assumptions made about rST in the sphinx-gallery code...that may be a big trickier.

Everything we write is RST-formatted. Can you be more specific about what we'd need to reformat? For example it doesn't seem like a problem if we still write plot_whatever.rst as the output of gen_rst and readme.rst as the output of gen_gallery.rst.

@choldgraf
Copy link
Contributor

Well I'd think the points where we'd need to support markdown are:

  • When .py files are converted to .rst files on-disk (these would need to be .md)
  • In generating the .ipynb files (where we currently convert rst to markdown, we'd instead skip that step)

All the auto-generated files would still work fine, since MyST just adds a markdown parser, but Sphinx still retains the rST parser so those files work fine.

@lucyleeow
Copy link
Contributor

MyST discussion continued in #710

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

Successfully merging a pull request may close this issue.

9 participants