#25305 closed enhancement (fixed)
Adding has_graphviz, has_ffmpeg, has_imagemagick to sage.doctest.external
Reported by:  slabbe  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  doctest framework  Keywords:  thursdaysbdx 
Cc:  Merged in:  
Authors:  Sébastien Labbé  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  ba9cf98 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Implement this:
sage: from sage.doctest.external import has_graphviz, has_ffmpeg, has_imagemagick sage: has_graphviz() # random True sage: has_ffmpeg() # random True sage: has_imagemagick() # random True
so that graphviz, ffmpeg, imagemagick are included in the external software to be detected:
$ sage t optional=external,sage src/sage/graphs/graph_latex.py src/sage/plot/animate.py ... Using optional=external External software to be detected: cplex,ffmpeg,graphviz,gurobi, imagemagick,internet,latex,macaulay2,magma,maple,mathematica, matlab,octave,scilab ...
Change History (32)
comment:1 Changed 4 years ago by
 Branch set to u/slabbe/25305
 Commit set to 99f37acd186afa3ec558e86935fc4e6976f9a9b0
 Status changed from new to needs_review
 Summary changed from Adding method sage.external.has_graphviz to Adding method sage.doctest.external.has_graphviz
comment:2 Changed 4 years ago by
 Summary changed from Adding method sage.doctest.external.has_graphviz to Adding sage.doctest.external.has_graphviz
comment:3 Changed 4 years ago by
 Commit changed from 99f37acd186afa3ec558e86935fc4e6976f9a9b0 to e09d9b7b0a1c3e4c0614bf8639b0ae1252ea220c
Branch pushed to git repo; I updated commit sha1. New commits:
e09d9b7  25305: adding has_ffmpeg and has_imagemagick to sage.doctest.external

comment:4 Changed 4 years ago by
I added has_ffmpeg
and have_imagemagick
in the second commit.
For references, the list tests_not_run
below contains all tags of tests that are not run by default. The list was obtained from grep "not run"
on the log of ./sage t p all long showskipped
:
sage: tests_not_run = ['4ti2', 'and', 'axiom', 'beautifulsoup', 'benzene', 'bliss', 'buckygen', ....: 'cbc', 'chomp', 'coxeter3', 'cplex', 'cryptominisat', 'csdp', 'cunningham', ....: 'database', 'database_cremona_ellcurve', 'database_gap', ....: 'database_jones_numfield', 'database_kohel', 'database_mutation_class', ....: 'database_odlyzko_zeta', 'database_pari', 'database_stein_watkins', ....: 'database_symbolic_data', 'debug', 'fes', 'ffmpeg', 'fricas', 'frobby', ....: 'gambit', 'gap3', 'gap_packages', 'gdb', 'giacpy_sage', 'ginv', 'glucose', ....: 'gmp', 'gmpy2', 'gnuplot', 'gperftools', 'graphviz', 'growing', 'gurobi', ....: 'hmisc', 'imagemagick', 'internet', 'java', 'kash', 'latex', 'latte_int', ....: 'libbraiding', 'libhomfly', 'libjpeg', 'lie', 'lrs', 'lrslib', 'macaulay2', ....: 'magma', 'maple', 'mathematica', 'mathematicafrontend', 'matlab', 'mcqd', ....: 'meataxe', 'mupad', 'nonexistent_lp_solver', 'octave', 'ore_algebra', 'phc', ....: 'plantri', 'polymake', 'polytopes_db_4d', 'py3', 'pynormaliz', 'python_igraph', ....: 'qepcad', 'randomly', 'required', 'requires', 'rgraphics', 'rsat', 'runsnake', ....: 'scilab', 'sirocco', 'sloane_database', 'surf', 'tdlib', 'the', 'tides', ....: 'time', 'topcom', 'webbrowser']
And this is a tentative list of all "external" packages:
sage: S = set(package_versions('standard')) sage: O = set(package_versions('optional')) sage: E = set(package_versions('experimental')) sage: set(tests_not_run)  (SOE) {'and', 'axiom', 'beautifulsoup', 'chomp', 'cplex', 'cunningham', 'database', 'debug', 'fes', 'ffmpeg', 'ginv', 'glucose', 'gnuplot', 'gperftools', 'graphviz', 'growing', 'gurobi', 'hmisc', 'imagemagick', 'internet', 'java', 'kash', 'latex', 'libjpeg', 'lrs', 'macaulay2', 'magma', 'maple', 'mathematica', 'mathematicafrontend', 'matlab', 'mupad', 'nonexistent_lp_solver', 'octave', 'phc', 'polytopes_db_4d', 'py3', 'randomly', 'required', 'requires', 'rgraphics', 'rsat', 'runsnake', 'scilab', 'sloane_database', 'the', 'time', 'webbrowser'}
Maybe we want to detect some more external packages...
comment:5 Changed 4 years ago by
 Summary changed from Adding sage.doctest.external.has_graphviz to Adding has_graphviz, has_ffmpeg, has_imagemagick to sage.doctest.external
comment:6 Changed 4 years ago by
 Commit changed from e09d9b7b0a1c3e4c0614bf8639b0ae1252ea220c to fe6163f0d36ea1e36588e3eff0101024dc0dd4b2
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
fe6163f  25305: adding has_ffmpeg and has_imagemagick to sage.doctest.external

comment:7 Changed 4 years ago by
 Description modified (diff)
comment:8 Changed 4 years ago by
 Description modified (diff)
comment:9 followups: ↓ 10 ↓ 13 Changed 4 years ago by
Shouldn't this use the recently merged #20382?
comment:10 in reply to: ↑ 9 ; followup: ↓ 14 Changed 4 years ago by
Replying to vdelecroix:
Shouldn't this use the recently merged #20382?
Actually, I think that all the has_xxx
stuff would better be features.
comment:11 Changed 4 years ago by
 Description modified (diff)
comment:12 Changed 4 years ago by
 Branch changed from u/slabbe/25305 to u/slabbe/25305usingfeatures
 Commit changed from fe6163f0d36ea1e36588e3eff0101024dc0dd4b2 to cb6e0887112a2cd8be4233ba36961a37b20cef41
New commits:
cb6e088  25305: adding has_ffmpeg, has_imagemagick, has_graphviz to sage.doctest.external

comment:13 in reply to: ↑ 9 Changed 4 years ago by
Replying to vdelecroix:
Shouldn't this use the recently merged #20382?
Ok as you wish. I am now using find_executable
from the Features
stuff instead of the have_program
from the misc
stuff.
comment:14 in reply to: ↑ 10 ; followup: ↓ 15 Changed 4 years ago by
Replying to vdelecroix:
Actually, I think that all the
has_xxx
stuff would better be features.
Why better? To me adding three new has_key
functions to the external.py
module makes things better. But changing has_maple
in a way that it uses features
does not make anything better.
The way the external.py
module is coded right now is that the presence of an external software with name tag
is detected with a method whose name is has_tag
. The goal of this ticket is not to redesign all of this external.py
module. So I suggest that further uses of feature inside of external.py
be postpone to another ticket if this is at all pertinent.
Needs review!
comment:15 in reply to: ↑ 14 Changed 4 years ago by
 Keywords thursdaysbdx added
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to needs_info
Replying to slabbe:
Replying to vdelecroix:
Actually, I think that all the
has_xxx
stuff would better be features.Why better? To me adding three new
has_key
functions to theexternal.py
module makes things better. But changinghas_maple
in a way that it usesfeatures
does not make anything better.
I just realized that it is not possible for has_maple
. The maple interface allows subtle configurations such as
 an alternative name for the executable
 a remote execution through ssh
This is not the kind of things a feature is aware of.
The way the
external.py
module is coded right now is that the presence of an external software with nametag
is detected with a method whose name ishas_tag
. The goal of this ticket is not to redesign all of thisexternal.py
module. So I suggest that further uses of feature inside ofexternal.py
be postpone to another ticket if this is at all pertinent.
In your situation, you are just checking for the presence of executable in the path. This is exactly what sage.features.Executable
is about. The main difference between `has_XXX_ and a feature is that
 a feature is not only about doctest
 a feature can run small tests
 meaningful error messages
 this is not run twice in the same session
What I wanted to suggest with my message is to introduce sage.features.graphviz
, sage.features.ffmpeg
and sage.features.imagemagick
and simply use
def has_ffmpeg(): from sage.features.ffmpeg import FFmpeg return FFmpeg.is_present()
The software FFmpeg
is indeed used in other parts of Sage (e.g. animate
) and it would help to have it as a feature.
comment:16 Changed 4 years ago by
 Branch changed from u/slabbe/25305usingfeatures to u/slabbe/25305withExecutableclass
 Commit changed from cb6e0887112a2cd8be4233ba36961a37b20cef41 to 034678bda18e59ebfff62dce7c813ad6ef249147
 Status changed from needs_info to needs_review
I now creates the files for ffmpeg, imagemagick and graphviz in the feature module and I am using these to solve the problem described in this ticket.
I suggest that the use of the FFmpeg().require()
in animate.py
etc be postpone to an indenpendent ticket.
New commits:
034678b  25305: adding has_ffmpeg, has_imagemagick, has_graphviz to sage.doctest.external

comment:17 Changed 4 years ago by
 Commit changed from 034678bda18e59ebfff62dce7c813ad6ef249147 to 9686e9705b77420610a6ed978392bafd8240690a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9686e97  25305: adding has_ffmpeg, has_imagemagick, has_graphviz to sage.doctest.external

comment:18 Changed 4 years ago by
 Commit changed from 9686e9705b77420610a6ed978392bafd8240690a to eeaa92f39f423a879e68bd7f0c35f11e563befeb
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
eeaa92f  25305: adding has_ffmpeg, has_imagemagick, has_graphviz to sage.doctest.external

comment:19 Changed 4 years ago by
I fixed some doctests and removed import os,subprocess
in the previous two forced pushed.
comment:20 Changed 4 years ago by
Needs review!
comment:21 Changed 4 years ago by
 Status changed from needs_review to needs_work
Image magick is a suite and not a unique executable:
magick
magickscript
convert
mogrify
identify
composite
montage
compare
 etc
comment:22 Changed 4 years ago by
Can you provide an example where a command from imagemagick other than convert
is used in the sage source code?
$ sage grep mogrify $ sage grep magickscript $ sage grep magick sage/misc/latex.py: http://www.imagemagick.org sage/misc/latex.py: print("http://www.imagemagick.org to download these programs.") sage/misc/latex.py: print("Go to http://www.imagemagick.org to download it.") sage/misc/latex.py: print("Go to http://www.imagemagick.org to download it.") sage/combinat/words/paths.py: See www.imagemagick.org, for example. sage/graphs/graph_latex.py: convert: http://www.imagemagick.org (the ImageMagick suite) sage/plot/animate.py: `ImageMagick <http://www.imagemagick.org>`_ sage/plot/animate.py: See www.imagemagick.org and www.ffmpeg.org for more information. sage/plot/animate.py:  `ImageMagick <http://www.imagemagick.org>`_ sage/plot/animate.py:See www.imagemagick.org and www.ffmpeg.org for more information.""" sage/plot/animate.py:See www.imagemagick.org and www.ffmpeg.org for more information.""" sage/plot/animate.py: See www.imagemagick.org and www.ffmpeg.org for more information. etc.
comment:23 Changed 4 years ago by
 Commit changed from eeaa92f39f423a879e68bd7f0c35f11e563befeb to cb740337eb81fbdf00180ffececca0cef2edb875
comment:24 Changed 4 years ago by
 Status changed from needs_work to needs_review
Branch rebased on top of 8.3.beta2.
Added doc about the fact that currently only the presence of convert is checked.
I also fix the doctest that was failing on the patchbot.
Needs review.
comment:25 Changed 4 years ago by
You forgot the copyright.
The rest looks ok.
comment:26 Changed 4 years ago by
 Status changed from needs_review to needs_work
comment:27 Changed 4 years ago by
 Commit changed from cb740337eb81fbdf00180ffececca0cef2edb875 to ba9cf98f587a0304642946797b995f6790835aad
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d4c5fec  25305: adding has_ffmpeg, has_imagemagick, has_graphviz to sage.doctest.external

8712542  25305: added doc about only convert being currently checked

baa0bae  25305: fix doctests

ba9cf98  25305: added licence to newly created files

comment:28 Changed 4 years ago by
 Status changed from needs_work to needs_review
Rebased the branch on top of 8.3.beta4.
Added a commit which adds the licence to the newly created files.
Needs review.
comment:29 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:30 Changed 4 years ago by
Thank you for the review
comment:31 Changed 4 years ago by
 Branch changed from u/slabbe/25305withExecutableclass to ba9cf98f587a0304642946797b995f6790835aad
 Resolution set to fixed
 Status changed from positive_review to closed
comment:32 Changed 4 years ago by
 Commit ba9cf98f587a0304642946797b995f6790835aad deleted
I noticed that some of the doctests have
# optional  graphviz
while some other doctests have
# optional: graphviz
Are both styles equivalent?
New commits:
25305: adding sage.doctest.external.has_graphviz