Opened 5 years ago

Closed 5 years ago

#23983 closed defect (fixed)

dot2tex breaks docbuild

Reported by: Jeroen Demeyer Owned by:
Priority: blocker Milestone: sage-8.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:

Status badges

Description (last modified by Jeroen Demeyer)

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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/site-packages/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/sage-patchbot/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main
[dochtml]     "__main__", fname, loader, pkg_name)
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/runpy.py", line 72, in _run_code
[dochtml]     exec code in run_globals
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
[dochtml]     main()
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1675, in main
[dochtml]     builder()
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 310, in _wrapper
[dochtml]     getattr(get_builder(document), 'inventory')(*args, **kwds)
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 505, in _wrapper
[dochtml]     build_many(build_ref_doc, L)
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 260, in build_many
[dochtml]     results.append(target(arg))
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 71, in build_ref_doc
[dochtml]     getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 720, in _wrapper
[dochtml]     getattr(DocBuilder, build_type)(self, *args, **kwds)
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 105, in f
[dochtml]     runsphinx()
[dochtml]   File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/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 Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

comment:3 Changed 5 years ago by Jeroen Demeyer

Branch: u/jdemeyer/dot2tex_breaks_docbuild

comment:4 Changed 5 years ago by Jeroen Demeyer

Authors: Jeroen Demeyer
Cc: Nicolas M. Thiéry added
Commit: 899b06a3f1b420c59067ee9f463901c1a7c04df9
Status: newneeds_review

New commits:

899b06aExplicitly check for Graphviz in have_dot2tex()

comment:5 Changed 5 years ago by Maarten Derickx

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.

Last edited 5 years ago by Maarten Derickx (previous) (diff)

comment:6 Changed 5 years ago by Maarten Derickx

P.s. the experimental package graphiz still builds on OS X but apparently not on other platforms, see #7438 .

comment:7 Changed 5 years ago by Travis Scrimshaw

We basically require a system-wide 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 in reply to:  7 Changed 5 years ago by Martin Rubey

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 Travis Scrimshaw

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 Jeroen Demeyer

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 Jeroen Demeyer

Description: modified (diff)

comment:12 Changed 5 years ago by Jeroen Demeyer

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 Maarten Derickx

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 Maarten Derickx

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 git

Commit: 899b06a3f1b420c59067ee9f463901c1a7c04df913a60ea63818302d376a1aa44fbb6faf4f4e28f3

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

13a60eaCheck for Graphviz dependency when installing dot2tex

comment:16 Changed 5 years ago by Maarten Derickx

Reviewers: Maarten Derickx
Status: needs_reviewpositive_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.

Last edited 5 years ago by Maarten Derickx (previous) (diff)

comment:17 Changed 5 years ago by Travis Scrimshaw

Typo? http://www.graphviz.org/Download..php"

comment:18 Changed 5 years ago by Maarten Derickx

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 Volker Braun

Branch: u/jdemeyer/dot2tex_breaks_docbuild13a60ea63818302d376a1aa44fbb6faf4f4e28f3
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.