Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uses deprecated and now removed sphinx.ext.mathbase #451

Closed
t-b opened this issue Aug 17, 2019 · 11 comments
Closed

Uses deprecated and now removed sphinx.ext.mathbase #451

t-b opened this issue Aug 17, 2019 · 11 comments
Assignees
Labels
bug Problem in existing code code Source code

Comments

@t-b
Copy link
Contributor

t-b commented Aug 17, 2019

Using latest sphinx 47cd262b (Merge branch '2.0', 2019-08-01) breathe breaks due to

Exception occurred:
  File "/home/firma/devel/breathe/breathe/node_factory.py", line 5, in <module>
    import sphinx.ext.mathbase
ModuleNotFoundError: No module named 'sphinx.ext.mathbase'
$git grep mathbase
breathe/node_factory.py:import sphinx.ext.mathbase
breathe/node_factory.py:    math_nodes.displaymath = sphinx.ext.mathbase.displaymath
breathe/renderer/sphinxrenderer.py:            # Here we steal the core of the mathbase "math" directive handling code from:
breathe/renderer/sphinxrenderer.py:            #    sphinx.ext.mathbase
documentation/source/latexmath.rst:``sphinx.ext.mathbase`` which is then picked up and rendered by the extension

I'm not sure how to fix that.

@vermeeren
Copy link
Collaborator

Quoting from the math extension doc[1]:

Changed in version 1.8: Math support for non-HTML builders is integrated to sphinx-core. So mathbase extension is no longer needed.

Looking at mathbase's source in Sphinx 2.2.0[2] confirms this. The deprecation messages also indicate the replacement function that should be used. Updating the import and function names in Breathe should be all that is required to fix this, if no complications show up.

[1] https://www.sphinx-doc.org/en/master/usage/extensions/math.html
[2] https://github.com/sphinx-doc/sphinx/blob/v2.2.0/sphinx/ext/mathbase.py

@vermeeren
Copy link
Collaborator

#469 fixes (at least part) of this. @t-b Could you confirm with sphinx master? If all is good, I'll probably release this as a bugfix update very soon.

@t-b
Copy link
Contributor Author

t-b commented Jan 14, 2020

@melvinvermeeren unfortunately this does not suffice for sphinx master/v2.3.1 anymore.

I'm getting

======================================================================
ERROR: test_renderer.test_render_define_no_initializer
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/firma/devel/sphinx/sphinx/environment/__init__.py", line 515, in get_domain
    return self.domains[domainname]
KeyError: 'c'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/firma/.pyenv/versions/3.6.7/lib/python3.6/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/firma/devel/breathe/tests/test_renderer.py", line 382, in test_render_define_no_initializer
    signature = find_node(render(member_def), 'desc_signature')
  File "/home/firma/devel/breathe/tests/test_renderer.py", line 257, in render
    return renderer.render(member_def)
  File "/home/firma/devel/breathe/breathe/renderer/sphinxrenderer.py", line 1308, in render
    result = method(self, node)
  File "/home/firma/devel/breathe/breathe/renderer/sphinxrenderer.py", line 1253, in dispatch_memberdef
    return self.visit_define(node)
  File "/home/firma/devel/breathe/breathe/renderer/sphinxrenderer.py", line 1050, in visit_define
    return self.render_declaration(node, declaration, update_signature=update_define_signature)
  File "/home/firma/devel/breathe/breathe/renderer/sphinxrenderer.py", line 412, in render_declaration
    nodes = self.run_domain_directive(obj_type, [declaration.replace('\n', ' ')])
  File "/home/firma/devel/breathe/breathe/renderer/sphinxrenderer.py", line 357, in run_domain_directive
    nodes = domain_directive.run()
  File "/home/firma/devel/sphinx/sphinx/directives/__init__.py", line 187, in run
    self.add_target_and_index(name, sig, signode)
  File "/home/firma/devel/sphinx/sphinx/domains/c.py", line 209, in add_target_and_index
    domain = cast(CDomain, self.env.get_domain('c'))
  File "/home/firma/devel/sphinx/sphinx/environment/__init__.py", line 517, in get_domain
    raise ExtensionError(__('Domain %r is not registered') % domainname)
sphinx.errors.ExtensionError: Domain 'c' is not registered

----------------------------------------------------------------------
Ran 18 tests in 0.294s

FAILED (errors=4)
Makefile:24: die Regel für Ziel „test“ scheiterte
make: *** [test] Fehler 1

from make tests of version 27e1fcd0 (changelog: update for #469, 2020-01-14).

@vermeeren
Copy link
Collaborator

At a glance this seems like a different issue, appears the c domain does not exist or is not loaded properly in that output. Comment #469 (comment) does suggest things should work with sphinx master.

Are you sure the paths and/or installations are healthy?

@t-b
Copy link
Contributor Author

t-b commented Jan 14, 2020

@melvinvermeeren I've created #471 to check but have you also seen #465?

@rscohn2
Copy link
Contributor

rscohn2 commented Jan 15, 2020

It looks like I was not running unit tests correctly and saw the problem with math, but did not see this problem. When I get some time to work on the sectioning again, I can look at this, but it won't be in the next week.

@vermeeren
Copy link
Collaborator

vermeeren commented Jan 15, 2020

Yeah this definitely is the same as #465, I remember that in the past tests also needed updating due to changes in Sphinx. For example 22a8880 and 7c0a1ed.

Not sure how to handle this one though. I see Sphinx has a add_domain() interface.
https://github.com/sphinx-doc/sphinx/blob/v2.3.1/sphinx/registry.py#L161

But clearly, the C domain inside Sphinx should be calling it, as shown here.
https://github.com/sphinx-doc/sphinx/blob/v2.3.1/sphinx/domains/c.py#L329

So somehow the C domain is not being loaded. Could be Breathe is missing some initialisation statements, or Sphinx changed in such a way that the C domain is no longer loaded by default. Where to look next is not obvious to me.

@vermeeren
Copy link
Collaborator

To add to the above: The CI on #469, and current Breathe master, does pass properly. So I feel like it is safe to say #469's patchset is correct and this is a distinct issue.

@t-b
Copy link
Contributor Author

t-b commented Feb 1, 2020

This is fixed in #469, the domain errors are tracked in #465, and the PR is #472.

@t-b t-b closed this as completed Feb 1, 2020
@vermeeren vermeeren self-assigned this Feb 2, 2020
@vermeeren
Copy link
Collaborator

Thanks for the status updates, I'll handle the open MRs now.

@vermeeren
Copy link
Collaborator

Breathe v4.14.1 is now released with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem in existing code code Source code
Projects
None yet
Development

No branches or pull requests

3 participants