#25537 closed defect (fixed)

sage -rst2ipynb should provide a usefull message if rst2ipynb is not installed

Reported by: slabbe Owned by:
Priority: major Milestone: sage-8.3
Component: scripts Keywords: thursdaysbdx
Cc: tmonteil Merged in:
Authors: Sébastien Labbé, Thierry Monteil Reviewers: Thierry Monteil, Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: da7f22b (Commits) Commit: da7f22b444281e0c61253a68d929512732bcf44f
Dependencies: #23416 Stopgaps:

Description

I get:

$ sage -rst2ipynb yo.rst yo.ipynb
/home/slabbe/GitBox/sage/src/bin/sage: ligne 775 : exec: rst2ipynb : non trouvé

???

Change History (15)

comment:1 Changed 17 months ago by slabbe

It seems to be the normal behavior when an optional package is not installed:

slabbe@miami bin $ sage -polymake
/home/slabbe/GitBox/sage/src/bin/sage: ligne 524 : exec: polymake : non trouvé
slabbe@miami bin $ sage -kash
/home/slabbe/GitBox/sage/src/bin/sage: ligne 559 : exec: kash : non trouvé

comment:2 Changed 17 months ago by slabbe

When kash, polymake, etc. are not installed, the doc of sage -advanced says:

  -kash [...]         -- run Sage's Kash with given arguments
                         (not installed currently, run sage -i kash)
  -lisp [...]         -- run Lisp interpreter included with Sage
  -M2 [...]           -- run Sage's Macaulay2 with given arguments
                         (not installed currently, run sage -i macaulay2)
  -maxima [...]       -- run Sage's Maxima with given arguments
  -mwrank [...]       -- run Sage's mwrank with given arguments
  -polymake [...]     -- run Sage's Polymake with given arguments
                         (not installed currently, run sage -i polymake)

comment:3 Changed 17 months ago by slabbe

  • Branch set to u/slabbe/25537
  • Commit set to 459e5d3891498195bc8789073ae9d1ad7d266e16
  • Dependencies set to #23416
  • Status changed from new to needs_review

I changed sage -advanced for rst2ipynb to do like the others kash and polymake, etc.


Last 10 new commits:

2af3bfc#23416 : let nbconvert write file and save images
aa75e08#23416 : postprocessing
03e157cMerge branch 'u/tmonteil/provide_a__sage__ipynb2rst__command' of trac.sagemath.org:sage into HEAD
0785497#23416 : command line doctest
5ed5ab1#23416 : remove depedency test to pandoc
7897736#23416 : typo
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
459e5d325537: add doc to install rst2ipynb in sage -advanced

comment:4 Changed 17 months ago by slabbe

  • Keywords thursdaysbdx added

comment:5 Changed 17 months ago by slabbe

  • Cc tmonteil added

comment:6 Changed 17 months ago by tmonteil

  • Reviewers set to Thierry Monteil

The code you provide does not fix the issue mentioned in the title and the description.

comment:7 Changed 17 months ago by slabbe

Yes I know. Do you know how we could give better error messages to the user when not installed? You know bash more than me...

comment:8 Changed 17 months ago by tmonteil

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

comment:9 Changed 17 months ago by tmonteil

  • Commit changed from 459e5d3891498195bc8789073ae9d1ad7d266e16 to da7f22b444281e0c61253a68d929512732bcf44f

Here is some code that does the job, but i am not sure this makes consensus, regarding the other cases in the same script.


New commits:

8456392Merge branch 'u/slabbe/25537' of trac.sagemath.org:sage into HEAD
da7f22b#25537 : trigger a message when rst2ipynb is not installed, comment 7.

comment:10 follow-up: Changed 16 months ago by slabbe

  • Authors set to Sébastien Labbé, Thierry Monteil
  • Reviewers changed from Thierry Monteil to Thierry Monteil, Sébastien Labbé
  • Status changed from needs_review to needs_info

Do we lose something by removing the exec part ?

comment:11 Changed 15 months ago by slabbe

ping:) please can you answer my small question before I can give positive review?

comment:12 Changed 15 months ago by slabbe

ping

comment:13 in reply to: ↑ 10 Changed 15 months ago by tmonteil

Replying to slabbe:

Do we lose something by removing the exec part ?

The way i wrote it, i would say no. The trick with exec is that the current process is replaced, hence after the command is finished, the further lines of the current script are not executed. This explains why, when removing the exec i had to catch the error (to send the appropriate message) and then be sure to exit.

The only drawback i see, is that the construction is different from the other cases in the script (that still use exec).

comment:14 Changed 15 months ago by slabbe

  • Status changed from needs_info to positive_review

Thanks for the clarifications.

comment:15 Changed 15 months ago by vbraun

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