Opened 5 years ago
Closed 5 years ago
#23983 closed defect (fixed)
dot2tex breaks docbuild
Reported by:  Jeroen Demeyer  Owned by:  

Priority:  blocker  Milestone:  sage8.1 
Component:  packages: optional  Keywords:  
Cc:  Maarten Derickx, Nicolas M. Thiéry  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Maarten Derickx 
Report Upstream:  N/A  Work issues:  
Branch:  13a60ea (Commits, GitHub, GitLab)  Commit:  13a60ea63818302d376a1aa44fbb6faf4f4e28f3 
Dependencies:  Stopgaps: 
Description (last modified by )
If the package dot2tex
is installed, but Graphviz is not, the documentation fails to build (see log below)
For the same reason, the following optional doctest fails:
sage t src/sage/categories/loop_crystals.py ********************************************************************** File "src/sage/categories/loop_crystals.py", line 118, in sage.categories.loop_crystals.LoopCrystals.ParentMethods.digraph Failed example: G.latex_options() # optional  dot2tex Expected: LaTeX options for Digraph on 29 vertices: {...'edge_options': <function <lambda> at 0x...>,...} Got: LaTeX options for Digraph on 29 vertices: {} **********************************************************************
However, this test is not actually run by default because of #23997.
[dochtml] [combinat ] reading sources... [ 99%] sage/combinat/words/word_options [dochtml] [combinat ] reading sources... [ 99%] sage/combinat/words/words [dochtml] [combinat ] reading sources... [ 99%] sage/combinat/yang_baxter_graph [dochtml] [combinat ] reading sources... [100%] sage/rings/cfinite_sequence [dochtml] [combinat ] WARNING: Could not locate Graphviz binaries [dochtml] [combinat ] WARNING: Failed to create xdotdata. Is Graphviz installed? [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/__init__.py:docstring of sage.combinat:39: WARNING: undefined label: sage.coding (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/__init__.py:docstring of sage.combinat:40: WARNING: undefined label: sage.dynamics (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/__init__.py:docstring of sage.combinat:41: WARNING: undefined label: sage.graphs (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/__init__.py:docstring of sage.combinat.__init__:39: WARNING: undefined label: sage.coding (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/__init__.py:docstring of sage.combinat.__init__:40: WARNING: undefined label: sage.dynamics (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/__init__.py:docstring of sage.combinat.__init__:41: WARNING: undefined label: sage.graphs (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/algebraic_combinatorics.py:docstring of sage.combinat.algebraic_combinatorics:36: WARNING: undefined label: sage.algebras.catalog (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/algebraic_combinatorics.py:docstring of sage.combinat.algebraic_combinatorics:37: WARNING: undefined label: sage.groups.groups_catalog (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/algebraic_combinatorics.py:docstring of sage.combinat.algebraic_combinatorics:64: WARNING: undefined label: sage.algebras.free_zinbiel_algebra (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/src/doc/en/reference/combinat/sage/combinat/counting.rst:6: WARNING: undefined label: sage.databases.oeis (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/enumerated_sets.py:docstring of sage.combinat.enumerated_sets:59: WARNING: undefined label: sage.groups.perm_gps.permutation_groups_catalog (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/enumerated_sets.py:docstring of sage.combinat.enumerated_sets:84: WARNING: undefined label: sage.groups.matrix_gps.catalog (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/posets/__init__.py:docstring of sage.combinat.posets.__init__:23: WARNING: undefined label: sage.categories.category (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/quickref.py:docstring of sage.combinat.quickref:63: WARNING: undefined label: sage.groups.groups_catalog (if the link has no caption the label must precede a section header) [dochtml] [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/quickref.py:docstring of sage.combinat.quickref:70: WARNING: undefined label: sage.graphs (if the link has no caption the label must precede a section header) [dochtml] Error building the documentation. [dochtml] Traceback (most recent call last): [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main [dochtml] "__main__", fname, loader, pkg_name) [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/runpy.py", line 72, in _run_code [dochtml] exec code in run_globals [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__main__.py", line 2, in <module> [dochtml] main() [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 1675, in main [dochtml] builder() [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 310, in _wrapper [dochtml] getattr(get_builder(document), 'inventory')(*args, **kwds) [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 505, in _wrapper [dochtml] build_many(build_ref_doc, L) [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 260, in build_many [dochtml] results.append(target(arg)) [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 71, in build_ref_doc [dochtml] getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds) [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 720, in _wrapper [dochtml] getattr(DocBuilder, build_type)(self, *args, **kwds) [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/__init__.py", line 105, in f [dochtml] runsphinx() [dochtml] File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage_setup/docbuild/sphinxbuild.py", line 218, in runsphinx [dochtml] raise exception [dochtml] OSError: [combinat ] WARNING: Could not locate Graphviz binaries
Change History (19)
comment:1 Changed 5 years ago by
Description:  modified (diff) 

comment:2 Changed 5 years ago by
Description:  modified (diff) 

comment:3 Changed 5 years ago by
Branch:  → u/jdemeyer/dot2tex_breaks_docbuild 

comment:4 Changed 5 years ago by
Authors:  → Jeroen Demeyer 

Cc:  Nicolas M. Thiéry added 
Commit:  → 899b06a3f1b420c59067ee9f463901c1a7c04df9 
Status:  new → needs_review 
comment:5 Changed 5 years ago by
Wouldn't it be better to check for wether Graphviz is installed during the installation of dot2tex an raise an appropriate error telling the user to install graphviz if it is not? I find it a bit confusing that now after a succesfull:
sage i dot2tex
one still can have have_dot2tex()
return false.
comment:6 Changed 5 years ago by
P.s. the experimental package graphiz still builds on OS X but apparently not on other platforms, see #7438 .
comment:7 followup: 8 Changed 5 years ago by
We basically require a systemwide installation of Graphviz because I don't think anyone uses the broken(? and outdated?) experimental package. So Graphviz is a dependency, but not during the installation of dot2tex. I would be in favor of erroring out of a dot2tex installation if Graphviz is not installed.
comment:8 Changed 5 years ago by
Replying to tscrim:
I would be in favor of erroring out of a dot2tex installation if Graphviz is not installed.
Yes, please  together with a helpful error message! That would have saved me quite some time in the past.
comment:9 Changed 5 years ago by
I do not object to the change here as it matches the docstring. However, I think we should have the same changes in assert_have_dot2tex
.
comment:10 Changed 5 years ago by
I think the question is: could it ever make sense to run dot2tex
without graphviz? If there is some (even minor) usefulness to dot2tex without graphviz, then graphviz should not be a dependency of dot2tex.
comment:11 Changed 5 years ago by
Description:  modified (diff) 

comment:12 Changed 5 years ago by
The description on PyPI says "Dot2tex is a tool for converting graphs rendered by Graphviz to formats that can be used with LaTeX."
So I guess that one could consider graphviz to be a hard dependency.
comment:13 Changed 5 years ago by
I also checked the sage code on occurrences of dot2tex, and they where all related to set_latex_options
of graphs or things that render like graphs. The documentation of set_latex_options
also clearly state that graph layout programs have to be one of the graphviz layout programs.
It is in theory possible for another program then one from graphviz suite to generate dot files, however all things that I have seen outside of sage (like gprof2dot) all have graphviz as a dependency since the dot file is a standard created by graphviz and graphviz provides a lot of utilities for generating/manipulating dot files.
I am for a hard dependency as well since at the moment dot2tex in sage is completely useless without graphviz, and I think it will be very unlikely that use cases of the dot file format will pop up in the future that do not in some way or another require graphviz.
comment:14 Changed 5 years ago by
And to the above I would like to add that having a hard dependency will make the user experience of sage less confusing. Since I think:
sage i dot2tex Error: you should install graphviz from http://www.graphviz.org/Download..php
is a better user experience then sage i dot2tex
seemingly succeeding, but all the places in sage that say they require dot2tex not working as they should, or falling back to an implementation not using dot2tex .
comment:15 Changed 5 years ago by
Commit:  899b06a3f1b420c59067ee9f463901c1a7c04df9 → 13a60ea63818302d376a1aa44fbb6faf4f4e28f3 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
13a60ea  Check for Graphviz dependency when installing dot2tex

comment:16 Changed 5 years ago by
Reviewers:  → Maarten Derickx 

Status:  needs_review → positive_review 
I tested that with this ticket dot2tex fails to install if graphviz is not installed and that it correctly installs if graphviz is installed. The error message looks very clear.
comment:18 Changed 5 years ago by
Nope, I was surprised as well, but that is actually the link. It works without the double dot as well, but will then redirect you to the double dot version.
comment:19 Changed 5 years ago by
Branch:  u/jdemeyer/dot2tex_breaks_docbuild → 13a60ea63818302d376a1aa44fbb6faf4f4e28f3 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Explicitly check for Graphviz in have_dot2tex()