Opened 2 years ago

Closed 17 months ago

#23416 closed enhancement (fixed)

Provide a "sage -ipynb2rst" command

Reported by: tmonteil Owned by:
Priority: major Milestone: sage-8.3
Component: notebook Keywords: MathExp2018, days94, thursdaysbdx
Cc: vdelecroix, slabbe, mlapointe Merged in:
Authors: Thierry Monteil Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: 6e37aa2 (Commits) Commit: 6e37aa20c1cc19aaaf8648c90ffe42125f462107
Dependencies: Stopgaps:

Description (last modified by tmonteil)

With jupyter as the default notebook, we need to easily transform an .ipynb notebook back into some source code. This ticket adds a sage -ipynb2rst command.

A custom template is created to write code blocks as:

   sage: # blah
   ....: # blah

and to make mathematics formulas the default role (hence avoiding the clumsy :math: notation everywhere). A postprocess script fixes some issues.

Note that reviewers have to run ./configure before make to let this ticket work in order to copy rst_sage.tpl and postprocess.py into the local/share/sage/ext/nbconvert/ directory.

Change History (28)

comment:1 Changed 2 years ago by tmonteil

  • Branch set to u/tmonteil/provide_a__sage__ipynb2rst__command

comment:2 Changed 2 years ago by tmonteil

  • Cc vdelecroix slabbe added
  • Commit set to c18f853443fbaac1a5bdba14d80ec9267f496a3a
  • Status changed from new to needs_review

New commits:

c18f853#23416 sage -ipynb2rst command.

comment:3 Changed 2 years ago by vdelecroix

Using bash 4.4.12 on Archlinux

$ sage -ipynb2rst measure_simulation.ipynb 
/opt/sage/src/bin/sage-ipynb2rst: ligne 39: erreur de syntaxe près du symbole inattendu « ( »
/opt/sage/src/bin/sage-ipynb2rst: ligne 39: `        jupyter nbconvert --to rst --RSTExporter.template_path=[\'$SAGE_EXTCODE/nbconvert/\'] --RSTExporter.template_file='rst_sage.tpl' --stdout ${1} | sed 's/:math://g' > ${2} ( echo -e '\n' ; help ) ;;'

comment:4 Changed 2 years ago by git

  • Commit changed from c18f853443fbaac1a5bdba14d80ec9267f496a3a to 2ef220d185bfdf9eb8ac2b51e186bd0a65d2ebb7

Branch pushed to git repo; I updated commit sha1. New commits:

2ef220d#23416 : typo.

comment:5 Changed 2 years ago by tmonteil

Hmm, i copy/pasted too fast, sorry for the typo, this should work better now.

comment:6 Changed 19 months ago by tmonteil

  • Keywords mathexp2018 added

comment:7 Changed 19 months ago by git

  • Commit changed from 2ef220d185bfdf9eb8ac2b51e186bd0a65d2ebb7 to 4ecef2fd657f5a60281a747213388f603536914f

Branch pushed to git repo; I updated commit sha1. New commits:

1edaf59Merge branch 'u/tmonteil/provide_a__sage__ipynb2rst__command' of trac.sagemath.org:sage into HEAD
4ecef2f#23416 : better template (output correctly displayed)

comment:8 Changed 18 months ago by git

  • Commit changed from 4ecef2fd657f5a60281a747213388f603536914f to aa75e084f6a0660384cc3d8bb3fd2951963c82c6

Branch pushed to git repo; I updated commit sha1. New commits:

d22357e#23416 : do not start with a blank line
2af3bfc#23416 : let nbconvert write file and save images
aa75e08#23416 : postprocessing

comment:9 Changed 18 months ago by tmonteil

  • Cc mlapointe added
  • Description modified (diff)

We could always add improvements, but i am fine with the current behavior for a first version.

comment:10 Changed 18 months ago by vdelecroix

  • Keywords MathExp2018 added; mathexp2018 removed
  • Milestone changed from sage-8.1 to sage-8.3

comment:11 Changed 18 months ago by slabbe

  • Reviewers set to Sébastien Labbé
  • Status changed from needs_review to needs_work

Thierry, can you add a doctest here in src/sage/tests/cmdline.py please?

comment:12 Changed 18 months ago by git

  • Commit changed from aa75e084f6a0660384cc3d8bb3fd2951963c82c6 to 0785497a620a405b72f45ea602c7136f5006f661

Branch pushed to git repo; I updated commit sha1. New commits:

03e157cMerge branch 'u/tmonteil/provide_a__sage__ipynb2rst__command' of trac.sagemath.org:sage into HEAD
0785497#23416 : command line doctest

comment:13 Changed 18 months ago by tmonteil

  • Status changed from needs_work to needs_review

Done.

comment:14 Changed 18 months ago by jdemeyer

Typo: the header that define -> the header that defines

comment:15 Changed 18 months ago by jdemeyer

  • Status changed from needs_review to needs_work

I generally don't like checks of the form

# tests if dependencies are available
pandoc -v > /dev/null 2>&1 || fail 'sage -ipynb2rst requires pandoc; please install it on your system.'

because it is not at all obvious that the jupyter nbconvert command will correctly execute pandoc if and only if that check above works. For example, if the pandoc command is configurable (I have no idea whether it is, but it might be), then you are doing the wrong check. Also, maybe pandoc fails to run because of a different reason than it not being installed (say, it is missing some library).

So unless there is a good reason to add that check, I would prefer to remove it.

comment:16 Changed 18 months ago by git

  • Commit changed from 0785497a620a405b72f45ea602c7136f5006f661 to 5ed5ab10ffc002cbc8b0d9ee23c109e50b585991

Branch pushed to git repo; I updated commit sha1. New commits:

5ed5ab1#23416 : remove depedency test to pandoc

comment:17 Changed 18 months ago by git

  • Commit changed from 5ed5ab10ffc002cbc8b0d9ee23c109e50b585991 to 7897736d7ae2e4b38bee1f6396629982f03be1df

Branch pushed to git repo; I updated commit sha1. New commits:

7897736#23416 : typo

comment:18 Changed 18 months ago by tmonteil

  • Status changed from needs_work to needs_review

Done. Sorry, it seems i cannot refrain to ask for permission ;)

comment:19 Changed 18 months ago by slabbe

  • Status changed from needs_review to needs_info

sage -sws2rst file.sws puts images into a file_media folder. Should we follow the same idea here instead of putting the images in the .?

comment:20 follow-up: Changed 18 months ago by slabbe

Why

        sage: L = ["sage", "--ipynb2rst", input, output]
        sage: try:
        ....:     _ = test_executable(['pandoc', '--version'])
        ....: except OSError:
        ....:     print('True')
        ....: else:
        ....:     _ = test_executable(L)
        ....:     print(open(output, 'r').read() == t)
        True

instead of

sage: L = ["sage", "--ipynb2rst", input, output]
sage: _ = test_executable(L)                     # optional: pandoc
sage: print(open(output, 'r').read() == t)       # optional: pandoc
True

?

comment:21 in reply to: ↑ 20 ; follow-up: Changed 18 months ago by tmonteil

Replying to slabbe:

Why

        sage: L = ["sage", "--ipynb2rst", input, output]
        sage: try:
        ....:     _ = test_executable(['pandoc', '--version'])
        ....: except OSError:
        ....:     print('True')
        ....: else:
        ....:     _ = test_executable(L)
        ....:     print(open(output, 'r').read() == t)
        True

instead of

sage: L = ["sage", "--ipynb2rst", input, output]
sage: _ = test_executable(L)                     # optional: pandoc
sage: print(open(output, 'r').read() == t)       # optional: pandoc
True

?

Because pandoc is not an optional package but a system dependency, so the doctest will never be run (unless someone explicitely ask for it).

comment:22 in reply to: ↑ 21 Changed 18 months ago by vdelecroix

Replying to tmonteil:

Replying to slabbe:

Why

        sage: L = ["sage", "--ipynb2rst", input, output]
        sage: try:
        ....:     _ = test_executable(['pandoc', '--version'])
        ....: except OSError:
        ....:     print('True')
        ....: else:
        ....:     _ = test_executable(L)
        ....:     print(open(output, 'r').read() == t)
        True

instead of

sage: L = ["sage", "--ipynb2rst", input, output]
sage: _ = test_executable(L)                     # optional: pandoc
sage: print(open(output, 'r').read() == t)       # optional: pandoc
True

?

Because pandoc is not an optional package but a system dependency, so the doctest will never be run (unless someone explicitely ask for it).

you can add has_pandoc in sage.doctest.external so that it can be properly detected (see also #25305).

comment:23 Changed 18 months ago by tmonteil

Why not. Then, i will wait the next beta to avoid useless rebasing mess.

comment:24 Changed 17 months ago by git

  • Commit changed from 7897736d7ae2e4b38bee1f6396629982f03be1df to 6e37aa20c1cc19aaaf8648c90ffe42125f462107

Branch pushed to git repo; I updated commit sha1. New commits:

30806a2Merge branch 'u/tmonteil/provide_a__sage__ipynb2rst__command' of trac.sagemath.org:sage into HEAD
f935b04#23416 : has_pandoc doctest function
6e37aa2#23416 : update cmdline doctest

comment:25 Changed 17 months ago by tmonteil

  • Keywords days94 added
  • Status changed from needs_info to needs_review

comment:26 Changed 17 months ago by slabbe

  • Status changed from needs_review to positive_review

comment:27 Changed 17 months ago by slabbe

  • Keywords thursdaysbdx added

comment:28 Changed 17 months ago by vbraun

  • Branch changed from u/tmonteil/provide_a__sage__ipynb2rst__command to 6e37aa20c1cc19aaaf8648c90ffe42125f462107
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.