Opened 2 years ago

Closed 2 years ago

#21514 closed enhancement (fixed)

sage -rst2ipynb command

Reported by: tmonteil Owned by:
Priority: major Milestone: sage-7.4
Component: notebook Keywords: days79
Cc: slabbe, kcrisman Merged in:
Authors: Thierry Monteil Reviewers: Sébastien Labbé, Nicolas M. Thiéry
Report Upstream: N/A Work issues:
Branch: cc0d50b (Commits) Commit: cc0d50b82f084d5f76c986df6031a83d89b24b6e
Dependencies: #21513 Stopgaps:

Description (last modified by tmonteil)

The sage -rst2ipynb allows to easily transform rst source into ipynb worksheet.

Change History (45)

comment:1 Changed 2 years ago by tmonteil

  • Dependencies changed from 21513 to #21513

comment:2 Changed 2 years ago by slabbe

  • Cc slabbe added

comment:3 Changed 2 years ago by kcrisman

  • Cc kcrisman added

comment:4 Changed 2 years ago by nthiery

Ah, I had not noticed that there was no code yet here.

Thierry, do you have preliminary code?

comment:5 Changed 2 years ago by tmonteil

Sure, but since there were many dependencies, i prefered to push changes once the previous dependency is positively reviewed, instead of having tons of branches to merge and rebase.

comment:6 Changed 2 years ago by nthiery

Sure thing :-)

comment:7 Changed 2 years ago by tmonteil

  • Branch set to u/tmonteil/sage__rst2ipynb_command

comment:8 Changed 2 years ago by tmonteil

  • Commit set to 4be49e441fbefa6ef6174477d5bb2e805a3015e1
  • Status changed from new to needs_review

New commits:

45e561a#21513: package rst-to-ipynb
966a95d#21513 : rst_to_ipynb -> rst-to-ipynb
d44379dMerge branch 'develop' into t/21513/package_rst-to-ipynb
421388a21513: Refactor the rst2ipynb package to use the new pip-installable package, step 1: renaming the pkg directory
d5d568021513: Refactor the rst2ipynb package to use the new pip-installable package, remainder
4e3c1fd#21514 : sage-rst2ipynb script
4be49e4#21514 : sage -rst2ipynb command

comment:9 Changed 2 years ago by slabbe

Would it be possible to have some basic help message including examples:

$ sage -rst2ipynb -h
usage: rst2ipynb [-o OUTPUT] [-k KERNEL] [-v] [-d] [input]
rst2ipynb: error: unrecognized arguments: -h

like

$ sage -rst2sws -h
Usage: 

    sage -rst2sws [options] <source> <destination>

Generates Sage worksheet (.sws) from standalone reStructuredText source.

Example:

    sage -rst2sws file.rst file.sws

Remarks:

    About LaTeX:

        You can put LaTeX expression between backticks "`", dollar signs
        "$" or double dollar signs "$$". Math role is not supported yet
        (get involved!).

        Examples: `\\alpha`, $\\beta$ or $$\\gamma$$.

    About backslashes:

        In reStructuredText, backslashes must be escaped, otherwise they
        are ignored. This is not quite practical for Sage's purposes
        since backslashes are never escaped in the docstrings. If you
        write `\alpha`, $\beta$ or $$\gamma$$, add the line

            ..  escape-backslashes

        to the file and this script will consider all backslashes
        escaped. Alternatively, you may use the available options.

Options:
  -h, --help            show this help message and exit
  --escape-backslashes  Escape all backslashes in the input file before
                        execution. Default: disabled.
  --no-escape-backslashes
                        Disable escaping all backslashes in the input file
                        before execution. This overrides the presence of the
                        line '.. escape-backslashes' in the file, which can
                        also be used to escape all backslashes in the file.

using argparse or something?

comment:10 Changed 2 years ago by nthiery

I am doing some minor adjustments that I'll discuss now with the above with Sébastien

comment:11 Changed 2 years ago by nthiery

Thierry: do you have a strong preference for

    sage -rst2ipynb bla.rst bla.ipynb

rather than

    sage -rst2ipynb bla.rst -o bla.ipynb

?

If yes, I might as well update rst2ipynb to simplify the sage wrapper

comment:12 Changed 2 years ago by nthiery

Hmm, although that would take another round of updating the package ...

comment:13 Changed 2 years ago by nthiery

I was thinking about that because I believe Sébastien's suggestion would be best implemented in the rst2ipynb script.

comment:14 Changed 2 years ago by nthiery

  • Branch changed from u/tmonteil/sage__rst2ipynb_command to u/nthiery/sage__rst2ipynb_command

comment:15 Changed 2 years ago by nthiery

  • Commit changed from 4be49e441fbefa6ef6174477d5bb2e805a3015e1 to 08cba4be20bb87753f9351154e5745ddbc65d3aa

I have checked Thierry's changes and made minor improvements. I am fine with the current state.


New commits:

08cba4b21514: minor improvements to the error messages

comment:16 Changed 2 years ago by nthiery

I'll now try to grab Sébastien :-)

comment:17 Changed 2 years ago by git

  • Commit changed from 08cba4be20bb87753f9351154e5745ddbc65d3aa to efd5c0b50f4b9e064ce26cef9e6834d686c23197

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

efd5c0b21514: more explicit error messages

comment:18 Changed 2 years ago by nthiery

We just discussed with Sébastien (cf. previous commit).

Plan for the next version of rst2ipynb itself (in a later ticket)

  • add documentation upon rst2ipynb -h
  • use the syntax rst2ipynb input output for consistency with other docutils tools
  • support for Python prompts >>>>
  • update the Sage package, simplifying sage-rst2ipy accordingly (just pass the arguments as is)

comment:19 Changed 2 years ago by slabbe

IPython notebook worksheet -> Jupyter notebook

comment:20 Changed 2 years ago by git

  • Commit changed from efd5c0b50f4b9e064ce26cef9e6834d686c23197 to 017ecb7681c0159927cf8c5e6bf15391c651056e

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

017ecb721514: improved sage -h documentation upon Sébastien's request

comment:21 Changed 2 years ago by tmonteil

  • Branch changed from u/nthiery/sage__rst2ipynb_command to u/tmonteil/sage__rst2ipynb_command

comment:22 Changed 2 years ago by slabbe

  • Authors changed from Thierry Monteil to Thierry Monteil, Nicolas M. Thiéry
  • Commit changed from 017ecb7681c0159927cf8c5e6bf15391c651056e to c0fe2a0d2da14ac6241853acdaec63cc0caea2c9
  • Reviewers set to Sébastien Labbé
  • Status changed from needs_review to positive_review

New commits:

c0fe2a0#21514 : add an help message.

comment:23 Changed 2 years ago by slabbe

  • Status changed from positive_review to needs_review

There were simultaneous commits including one that is not reviewed. So I put it back to needs review.

comment:24 follow-up: Changed 2 years ago by tmonteil

  • @slabbe : i added a very similar help system than for the other commands.
  • @nthiery : you should not push branches after some reviewers made an explicit request to the author, i was working on my branch without internet connection dudring the lunch break. Especially for such minor requests, it doesn't worth knitting with merges.
  • @slabbe : i used IPython notebook because of the .ipynb extension, i am not sure what is the recommended name.
Last edited 2 years ago by tmonteil (previous) (diff)

comment:25 Changed 2 years ago by nthiery

  • Branch changed from u/tmonteil/sage__rst2ipynb_command to u/nthiery/sage__rst2ipynb_command

comment:26 Changed 2 years ago by nthiery

  • Commit changed from c0fe2a0d2da14ac6241853acdaec63cc0caea2c9 to 7820d581801974126abb0763905fd9805cfedd37

I merged the two branches.


New commits:

08cba4b21514: minor improvements to the error messages
efd5c0b21514: more explicit error messages
017ecb721514: improved sage -h documentation upon Sébastien's request
7820d58Merge branch 'u/tmonteil/sage__rst2ipynb_command' of trac.sagemath.org:sage into t/21514/sage__rst2ipynb_command

comment:27 Changed 2 years ago by nthiery

Sorry for the miscoordination; there was a unique occasion (Sage Days 79 are finishing just now) to sprint with Sébastien with fast face-to-face discussions to polish the details. But you are right, I should have asked for confirmation on your side.

For the record: I just pushed on the rst2ipynb github repo the following changes:

  • rst2ipynb -h documentation
  • experimental support for Python (continuation) prompts (Sébastien: please try and shake!)
  • support for rst2ipynb input output

I let you decide whether you prefer to include that in #21513 and update #21513 accordingly or leave that for later. In the first case, I'll do a new release of rst2ipynb. I'll be on and off the network the next three days, but should be able to squeeze that within 24 hours after notice.

Yeah, almost done with that. Thank you Thierry for the wrapping and Sébastien for the review!

comment:28 Changed 2 years ago by slabbe

Dear Thierry, Nicolas,

After thinking about this, I suggest to merge into Sage the commit 017ecb7681c0159927cf8c5e6bf15391c651056e which does not include Thierry's commit ​c0fe2a0. Indeed, I asked for some nice help sage -rst2ipynb -h message, but after reconsideration I believe this is not the role of the bash code to do this. For the same reason as sage -ipython -h will give you the same thing as ipython -h. Therefore, I suggest to postpone the better help message to another ticket which will consist in updating rst2ipynb to a new version.

I wait for your approval before to do any change on this ticket.

(Git question: no branch are pointing to the commit 017ecb7681, do I have to create a branch that links to this commit, or is the commit hash enough?)

comment:29 Changed 2 years ago by nthiery

I am fine with both options.

comment:30 Changed 2 years ago by slabbe

  • Branch changed from u/nthiery/sage__rst2ipynb_command to 017ecb7681c0159927cf8c5e6bf15391c651056e
  • Commit 7820d581801974126abb0763905fd9805cfedd37 deleted

I am replacing the branch name by the earlier commit 017ecb7681c0159927cf8c5e6bf15391c651056e as I am suggesting this commit to be merged into Sage for this ticket. Let see if it works.

I am still waiting for Thierry's approval before changing the status to positive review.

comment:31 Changed 2 years ago by slabbe

  • Keywords days79 added

comment:32 Changed 2 years ago by slabbe

  • Branch changed from 017ecb7681c0159927cf8c5e6bf15391c651056e to u/slabbe/21514
  • Commit set to caa90a6da08b682581bfdd025c56af3cf49ef7d1

I created a branch u/slabbe/21514 with commit 017ecb7681c015 merged into 7.5.beta4.

Sébastien


Last 10 new commits:

966a95d#21513 : rst_to_ipynb -> rst-to-ipynb
d44379dMerge branch 'develop' into t/21513/package_rst-to-ipynb
421388a21513: Refactor the rst2ipynb package to use the new pip-installable package, step 1: renaming the pkg directory
d5d568021513: Refactor the rst2ipynb package to use the new pip-installable package, remainder
4e3c1fd#21514 : sage-rst2ipynb script
4be49e4#21514 : sage -rst2ipynb command
08cba4b21514: minor improvements to the error messages
efd5c0b21514: more explicit error messages
017ecb721514: improved sage -h documentation upon Sébastien's request
caa90a6Merge commit '017ecb7681c0159927cf8c5e6bf15391c651056e' into 7.5.beta4

comment:33 in reply to: ↑ 24 Changed 2 years ago by slabbe

Replying to tmonteil:

  • @slabbe : i used IPython notebook because of the .ipynb extension, i am not sure what is the recommended name.

http://nbviewer.jupyter.org/ says "Jupyter Notebooks". I would be okay with "IPython Notebooks" but not "IPython worksheets".

comment:34 Changed 2 years ago by git

  • Commit changed from caa90a6da08b682581bfdd025c56af3cf49ef7d1 to 227cbbaadfca24933eb69e45829810595f07d2b0

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

227cbba21514: adding tests for rst2ipynb in tests/cmdline.py

comment:35 Changed 2 years ago by git

  • Commit changed from 227cbbaadfca24933eb69e45829810595f07d2b0 to 735ecd62910294af67652f2bc6920d97f64646b4

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

735ecd621514: typo

comment:36 Changed 2 years ago by slabbe

I added some doctests in tests/cmdline.py. To me this ticket is a positive review. I let you review the doctests I added.

Sébastien

comment:37 Changed 2 years ago by slabbe

I forgot to add # optional - rst2ipynb to the new doctests...

comment:38 Changed 2 years ago by git

  • Commit changed from 735ecd62910294af67652f2bc6920d97f64646b4 to 479c2c74975a5420eccb0bfc106d7060836aa25c

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

479c2c721514: adding comment # optional - rst2ipynb

comment:39 Changed 2 years ago by slabbe

Modifications to tests/cmdline.py needs review.

comment:40 Changed 2 years ago by slabbe

ping :)

comment:41 Changed 2 years ago by tmonteil

  • Branch changed from u/slabbe/21514 to u/tmonteil/21514

comment:42 Changed 2 years ago by slabbe

  • Commit changed from 479c2c74975a5420eccb0bfc106d7060836aa25c to cc0d50b82f084d5f76c986df6031a83d89b24b6e

ok for the merge:)


New commits:

c0fe2a0#21514 : add an help message.
cc0d50b21514 : merge.

comment:43 Changed 2 years ago by nthiery

Any reason left not to set a positive review?

comment:44 Changed 2 years ago by tmonteil

  • Authors changed from Thierry Monteil, Nicolas M. Thiéry to Thierry Monteil
  • Description modified (diff)
  • Reviewers changed from Sébastien Labbé to Sébastien Labbé, Nicolas M. Thiéry
  • Status changed from needs_review to positive_review

I had some general comments, but i guess it is better to have this merged soon.

comment:45 Changed 2 years ago by vbraun

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