Opened 3 years ago
Closed 3 years ago
#21514 closed enhancement (fixed)
sage rst2ipynb command
Reported by:  tmonteil  Owned by:  

Priority:  major  Milestone:  sage7.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 )
The sage rst2ipynb
allows to easily transform rst
source into ipynb
worksheet.
Change History (45)
comment:1 Changed 3 years ago by
 Dependencies changed from 21513 to #21513
comment:2 Changed 3 years ago by
 Cc slabbe added
comment:3 Changed 3 years ago by
 Cc kcrisman added
comment:4 Changed 3 years ago by
comment:5 Changed 3 years ago by
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 3 years ago by
Sure thing :)
comment:7 Changed 3 years ago by
 Branch set to u/tmonteil/sage__rst2ipynb_command
comment:8 Changed 3 years ago by
 Commit set to 4be49e441fbefa6ef6174477d5bb2e805a3015e1
 Status changed from new to needs_review
New commits:
45e561a  #21513: package rsttoipynb

966a95d  #21513 : rst_to_ipynb > rsttoipynb

d44379d  Merge branch 'develop' into t/21513/package_rsttoipynb

421388a  21513: Refactor the rst2ipynb package to use the new pipinstallable package, step 1: renaming the pkg directory

d5d5680  21513: Refactor the rst2ipynb package to use the new pipinstallable package, remainder

4e3c1fd  #21514 : sagerst2ipynb script

4be49e4  #21514 : sage rst2ipynb command

comment:9 Changed 3 years ago by
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 .. escapebackslashes 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 escapebackslashes Escape all backslashes in the input file before execution. Default: disabled. noescapebackslashes Disable escaping all backslashes in the input file before execution. This overrides the presence of the line '.. escapebackslashes' in the file, which can also be used to escape all backslashes in the file.
using argparse
or something?
comment:10 Changed 3 years ago by
I am doing some minor adjustments that I'll discuss now with the above with Sébastien
comment:11 Changed 3 years ago by
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 3 years ago by
Hmm, although that would take another round of updating the package ...
comment:13 Changed 3 years ago by
I was thinking about that because I believe Sébastien's suggestion would be best implemented in the rst2ipynb script.
comment:14 Changed 3 years ago by
 Branch changed from u/tmonteil/sage__rst2ipynb_command to u/nthiery/sage__rst2ipynb_command
comment:15 Changed 3 years ago by
 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:
08cba4b  21514: minor improvements to the error messages

comment:16 Changed 3 years ago by
I'll now try to grab Sébastien :)
comment:17 Changed 3 years ago by
 Commit changed from 08cba4be20bb87753f9351154e5745ddbc65d3aa to efd5c0b50f4b9e064ce26cef9e6834d686c23197
Branch pushed to git repo; I updated commit sha1. New commits:
efd5c0b  21514: more explicit error messages

comment:18 Changed 3 years ago by
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
sagerst2ipy
accordingly (just pass the arguments as is)
comment:19 Changed 3 years ago by
IPython notebook worksheet
> Jupyter notebook
comment:20 Changed 3 years ago by
 Commit changed from efd5c0b50f4b9e064ce26cef9e6834d686c23197 to 017ecb7681c0159927cf8c5e6bf15391c651056e
Branch pushed to git repo; I updated commit sha1. New commits:
017ecb7  21514: improved sage h documentation upon Sébastien's request

comment:21 Changed 3 years ago by
 Branch changed from u/nthiery/sage__rst2ipynb_command to u/tmonteil/sage__rst2ipynb_command
comment:22 Changed 3 years ago by
 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 3 years ago by
 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 followup: ↓ 33 Changed 3 years ago by
 @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.
comment:25 Changed 3 years ago by
 Branch changed from u/tmonteil/sage__rst2ipynb_command to u/nthiery/sage__rst2ipynb_command
comment:26 Changed 3 years ago by
 Commit changed from c0fe2a0d2da14ac6241853acdaec63cc0caea2c9 to 7820d581801974126abb0763905fd9805cfedd37
I merged the two branches.
New commits:
08cba4b  21514: minor improvements to the error messages

efd5c0b  21514: more explicit error messages

017ecb7  21514: improved sage h documentation upon Sébastien's request

7820d58  Merge branch 'u/tmonteil/sage__rst2ipynb_command' of trac.sagemath.org:sage into t/21514/sage__rst2ipynb_command

comment:27 Changed 3 years ago by
Sorry for the miscoordination; there was a unique occasion (Sage Days 79 are finishing just now) to sprint with Sébastien with fast facetoface 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 3 years ago by
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 3 years ago by
I am fine with both options.
comment:30 Changed 3 years ago by
 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 3 years ago by
 Keywords days79 added
comment:32 Changed 3 years ago by
 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 > rsttoipynb

d44379d  Merge branch 'develop' into t/21513/package_rsttoipynb

421388a  21513: Refactor the rst2ipynb package to use the new pipinstallable package, step 1: renaming the pkg directory

d5d5680  21513: Refactor the rst2ipynb package to use the new pipinstallable package, remainder

4e3c1fd  #21514 : sagerst2ipynb script

4be49e4  #21514 : sage rst2ipynb command

08cba4b  21514: minor improvements to the error messages

efd5c0b  21514: more explicit error messages

017ecb7  21514: improved sage h documentation upon Sébastien's request

caa90a6  Merge commit '017ecb7681c0159927cf8c5e6bf15391c651056e' into 7.5.beta4

comment:33 in reply to: ↑ 24 Changed 3 years ago by
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 3 years ago by
 Commit changed from caa90a6da08b682581bfdd025c56af3cf49ef7d1 to 227cbbaadfca24933eb69e45829810595f07d2b0
Branch pushed to git repo; I updated commit sha1. New commits:
227cbba  21514: adding tests for rst2ipynb in tests/cmdline.py

comment:35 Changed 3 years ago by
 Commit changed from 227cbbaadfca24933eb69e45829810595f07d2b0 to 735ecd62910294af67652f2bc6920d97f64646b4
Branch pushed to git repo; I updated commit sha1. New commits:
735ecd6  21514: typo

comment:36 Changed 3 years ago by
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 3 years ago by
I forgot to add # optional  rst2ipynb
to the new doctests...
comment:38 Changed 3 years ago by
 Commit changed from 735ecd62910294af67652f2bc6920d97f64646b4 to 479c2c74975a5420eccb0bfc106d7060836aa25c
Branch pushed to git repo; I updated commit sha1. New commits:
479c2c7  21514: adding comment # optional  rst2ipynb

comment:39 Changed 3 years ago by
Modifications to tests/cmdline.py
needs review.
comment:40 Changed 3 years ago by
ping :)
comment:41 Changed 3 years ago by
 Branch changed from u/slabbe/21514 to u/tmonteil/21514
comment:42 Changed 3 years ago by
 Commit changed from 479c2c74975a5420eccb0bfc106d7060836aa25c to cc0d50b82f084d5f76c986df6031a83d89b24b6e
comment:43 Changed 3 years ago by
Any reason left not to set a positive review?
comment:44 Changed 3 years ago by
 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 3 years ago by
 Branch changed from u/tmonteil/21514 to cc0d50b82f084d5f76c986df6031a83d89b24b6e
 Resolution set to fixed
 Status changed from positive_review to closed
Ah, I had not noticed that there was no code yet here.
Thierry, do you have preliminary code?