Opened 3 years ago
Closed 3 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 )
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 rst-to-ipynb
|
966a95d | #21513 : rst_to_ipynb -> rst-to-ipynb
|
d44379d | Merge branch 'develop' into t/21513/package_rst-to-ipynb
|
421388a | 21513: Refactor the rst2ipynb package to use the new pip-installable package, step 1: renaming the pkg directory
|
d5d5680 | 21513: 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 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 .. 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 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
sage-rst2ipy
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 follow-up: ↓ 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 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 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 -> rst-to-ipynb
|
d44379d | Merge branch 'develop' into t/21513/package_rst-to-ipynb
|
421388a | 21513: Refactor the rst2ipynb package to use the new pip-installable package, step 1: renaming the pkg directory
|
d5d5680 | 21513: Refactor the rst2ipynb package to use the new pip-installable package, remainder
|
4e3c1fd | #21514 : sage-rst2ipynb 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?