search

brechtm / rinohtype

The Python document processor
http://www.mos6581.org/rinohtype
GNU Affero General Public License v3.0
504 stars 61 forks source link

Crash when using graphviz directive #299

Closed rappdw closed 3 years ago

rappdw commented 3 years ago

repo to reproduce problem is here: github.com/rappdw/rinoh_error@graphviz

(log file is also in repo, but included here as well)

# Sphinx version: 4.2.0
# Python version: 3.9.7 (CPython)
# Docutils version: 0.17.1 release
# Jinja2 version: 3.0.1
# Last messages:
#   building [rinoh]: all documents
#   updating environment:
#   0 added, 0 changed, 0 removed
#   looking for now-outdated files...
#   none found
#   processing book...
#   index
#   chapter_1
#   bibliography
#   resolving references...
# Loaded extensions:
#   sphinx.ext.mathjax (4.2.0) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinx/ext/mathjax.py
#   sphinxcontrib.applehelp (1.0.2) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinxcontrib/applehelp/__init__.py
#   sphinxcontrib.devhelp (1.0.2) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinxcontrib/devhelp/__init__.py
#   sphinxcontrib.htmlhelp (2.0.0) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinxcontrib/htmlhelp/__init__.py
#   sphinxcontrib.serializinghtml (1.1.5) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinxcontrib/serializinghtml/__init__.py
#   sphinxcontrib.qthelp (1.0.3) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinxcontrib/qthelp/__init__.py
#   alabaster (0.7.12) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/alabaster/__init__.py
#   rinoh.frontend.sphinx (0.5.4) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/sphinx/__init__.py
#   myst_parser (0.15.2) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/myst_parser/__init__.py
#   sphinxcontrib.bibtex (2.4.2a0) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinxcontrib/bibtex/__init__.py
#   sphinx.ext.graphviz (4.2.0) from /Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinx/ext/graphviz.py
Traceback (most recent call last):
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 27, in map_node
    return cls._mapping[node_name.replace('-', '_')](node, **context)
KeyError: 'graphviz'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinx/cmd/build.py", line 280, in build_main
    app.build(args.force_all, filenames)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinx/application.py", line 343, in build
    self.builder.build_update()
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 290, in build_update
    self.build(['__all__'], to_build)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 357, in build
    self.write(docnames, list(updated_docnames), method)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/sphinx/__init__.py", line 251, in write
    self.write_document(entry)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/sphinx/__init__.py", line 257, in write_document
    rinoh_document = self.construct_rinohtype_document(data)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/sphinx/__init__.py", line 273, in construct_rinohtype_document
    rinoh_tree = from_doctree(doctree, sphinx_builder=self)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/rst/__init__.py", line 144, in from_doctree
    return mapped_tree.flowable()
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 132, in flowable
    flowable, = self.flowables()
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/rst/__init__.py", line 102, in flowables
    for flowable in super().flowables():
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 137, in flowables
    for i, flowable in enumerate(self.build_flowables()):
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 146, in build_flowables
    yield self.build_flowable()
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 162, in build_flowable
    return self.grouped_flowables_class(self.children_flowables(),
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 125, in children_flowables
    return list(chain(*(item.flowables() for item in children)))
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/rst/__init__.py", line 102, in flowables
    for flowable in super().flowables():
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 137, in flowables
    for i, flowable in enumerate(self.build_flowables()):
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 146, in build_flowables
    yield self.build_flowable()
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 162, in build_flowable
    return self.grouped_flowables_class(self.children_flowables(),
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 125, in children_flowables
    return list(chain(*(item.flowables() for item in children)))
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/rst/__init__.py", line 102, in flowables
    for flowable in super().flowables():
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 137, in flowables
    for i, flowable in enumerate(self.build_flowables()):
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 146, in build_flowables
    yield self.build_flowable()
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/sphinx/nodes.py", line 236, in build_flowable
    return super().build_flowable(id='%' + self.get('docname'), **kwargs)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 162, in build_flowable
    return self.grouped_flowables_class(self.children_flowables(),
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 125, in children_flowables
    return list(chain(*(item.flowables() for item in children)))
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/rst/__init__.py", line 102, in flowables
    for flowable in super().flowables():
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 137, in flowables
    for i, flowable in enumerate(self.build_flowables()):
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 146, in build_flowables
    yield self.build_flowable()
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 162, in build_flowable
    return self.grouped_flowables_class(self.children_flowables(),
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 124, in children_flowables
    children = self.getchildren()[skip_first:]
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 64, in getchildren
    return [self.map_node(child, **self.context)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 64, in <listcomp>
    return [self.map_node(child, **self.context)
  File "/Users/drapp/opt/miniconda3/envs/rinoh_error/lib/python3.9/site-packages/rinoh/frontend/__init__.py", line 29, in map_node
    filename, line, node_name = cls.node_location(node)
AttributeError: type object 'Section' has no attribute 'node_location'
rappdw commented 3 years ago

The sphinx Graphviz extension adds a graphviz node to the document tree (rather than image).

Adding the following to the sphinx conf.py will workaround the above issue. I suspect it requires some refinement to cover all cases.

Uncertain why the sphinx extension doesn't conform to the DocUtils dtd, and I'm not sure that this workaround would be the best way to handle things in general.


import rinoh as rt

from sphinx.ext.graphviz import render_dot
from rinoh.frontend.rst.nodes import Image

class Graphviz(Image):
    def __init__(self, doctree_node, **context):
        super().__init__(doctree_node, **context)
        self.code = doctree_node.attributes['code']

    def build_flowable(self):
        sphinx_app = self.context['sphinx_builder'].app
        options = self.options
        fname, outfn = render_dot(sphinx_app, self.code, options, sphinx_app.config.graphviz_output_format)
        return rt.Image(outfn, **options)
brechtm commented 3 years ago

Your code looks like a good way to implement support for the graphviz extension into rinohtype. If you can provide a PR (add the code to src/rinoh/frontend/sphinx/nodes.py, preferably including a regression test), I'm happy to merge it!

The sphinx Graphviz extension adds a graphviz node to the document tree (rather than image).

I see this approach used in many extensions, presumably because this is how things are documented in the Sphinx manual. It took me a while to realize that in most cases it's possible to make use of existing docutils/Sphinx nodes instead of creating new ones. And that's probably only because I'm developing a third-party Sphinx builder.

You could file an issue on the Sphinx issue tracker to bring this to the attention of the Sphinx developers, but I'm not sure how important refactoring this is to them.

The sphinxcontrib-drawio extension's implementation is a good reference for how this type of extension (image generation) can be implemented independent of the builders. For general remarks on Sphinx extensions, see:

rappdw commented 3 years ago

I'm trying to run the regression tests (following outline in DEVELOPING.rst) and most of the regressions tests are failing with the following error. I'm guessing I'm overlooking something simple, but it's not obvious to me. (Running on OSX, and I've brew install poppler.)


Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.9/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
  File "/usr/local/Cellar/python@3.9/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
  File "/Users/drapp/dev/rinohtype/tests_regression/helpers/diffpdf.py", line 127, in diff_page
    if compare_page(a_filename, b_filename, a_page, b_page):
  File "/Users/drapp/dev/rinohtype/tests_regression/helpers/diffpdf.py", line 152, in compare_page
    compare = Popen(MAGICK + ['compare', '-', '-metric', 'AE', 'null:'],
  File "/usr/local/Cellar/python@3.9/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/local/Cellar/python@3.9/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 1821, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'compare'```
brechtm commented 3 years ago

You also need ImageMagick (compare, convert, ...), which can also be installed by Homebrew.

I should check for ImageMagick in the diffpdf script and exit with a clear error message.

brechtm commented 3 years ago

I should check for ImageMagick in the diffpdf script and exit with a clear error message.

Added in 06e7490e316ba74470ccb3af548c101ea834126f.

Graphviz support was implemented in #300.