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

Sphinx 3.1.1 name clash prevents autodoc importing. #7841

Closed
oscarbenjamin opened this issue Jun 15, 2020 · 7 comments
Closed

Sphinx 3.1.1 name clash prevents autodoc importing. #7841

oscarbenjamin opened this issue Jun 15, 2020 · 7 comments

Comments

@oscarbenjamin
Copy link

Sphinx 3.1.1 seems to have changed the way that autodoc finds python imports which leads to two kinds of failure in building the sympy docs. Effectively the change is the difference between

from sympy.series.order import Order

compared to this

import sympy
Order = sympy.series.order

These are not equivalent and there are many cases in sympy where only the former works so autodoc fails to find many objects in sympy now.

This is a simple view of the relevant module structure in sympy:

sympy/
    __init__.py
    series/
        __init__.py
        order.py
        series.py

In order.py there is a class called Order and in series.py there is a function called series. The main sympy/init.py effectively does

# Import the series function
from .series.series import series

In this setup the Order class can be imported as

>>> from sympy.series.order import Order
>>> Order
<class 'sympy.series.order.Order'>

However it cannot be accessed through attributes from sympy e.g.:

>>> import sympy
>>> sympy.series.order.Order
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'function' object has no attribute 'order'
>>> sympy.series
<function series at 0x104fa4ca0>

This is the same error as seen when building the sphinx docs:

WARNING: autodoc: failed to import class 'series.order.Order' from
module 'sympy'; the following exception was raised:
Traceback (most recent call last):
  File "/Users/enojb/current/sympy/38venv/lib/python3.8/site-packages/sphinx/util/inspect.py",
line 324, in safe_getattr
    return getattr(obj, name, *defargs)
AttributeError: 'function' object has no attribute 'order'

This is because getattr is being used rather than importlib to locate the import. These are not generally equivalent and it seems that sphinx 3.1.1 has switched from using one to use the other.

There is another issue when attempting to reproduce this which is that there are many deprecated import objects in sympy/__init__.py and calling getattr on them triggers a deprecation warning:
sympy/sympy#19554

To Reproduce

Steps to reproduce the behavior:

$ git clone https://github.com/sympy/sympy.git
$ cd sympy
$ pip install mpmath
$ pip install -e .
$ cd docs
$ make html

Expected behavior
The docs would build successfully without warning like they do in 3.1.0

Your project

This was already mentioned on the sphinx mailing list:
https://groups.google.com/forum/#!msg/sphinx-users/mx83xLsDjBs/Qc3jW-z7AQAJ
Corresponding sympy issue:
sympy/sympy#19551

@tk0miya
Copy link
Member

tk0miya commented Jun 16, 2020

Hmm... this was introduced by #7815 for the fix of #7812. It's difficult to resolve both to me. It's terrible there are two components on the same python name. Ideally, .. automodule:: sympy.series should load the module named sympy.series, but .. autofunction:: sympy.series should load the function named sympy.series! That's very ambiguous.

Could you add a separator between the module name and object name like .. autoclass:: sympy.series.order::Order? Then autodoc can detect correct module name.

@oscarbenjamin
Copy link
Author

Thanks @tk0miya. That seems to have fixed it.

I thought that there should be some way to disambiguate the module from the object but I couldn't find anything in the docs:
https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html

@asmeurer
Copy link
Contributor

Shouldn't the use of automodule vs. autofunction be enough to disambiguate the two?

@tk0miya
Copy link
Member

tk0miya commented Jun 17, 2020

Indeed. How about .. autoclass:: foo.bar.baz.Qux? There are some interpretations of module name.

  1. Qux class on foo.bar.baz module.
  2. baz.Qux class (nested class) on foo.bar module.
  3. bar.baz.Qux class (nested class) on foo module.

Sphinx also describes methods, attributes and so on. To do that, we need to split a module name and an object name from the given string. And now it tries to check the attribute exists on the shallower module. Please let me know if you have better idea.

@tk0miya
Copy link
Member

tk0miya commented Jun 17, 2020

I thought that there should be some way to disambiguate the module from the object but I couldn't find anything in the docs:

Thanks, I did not notice that. I'll work it later.

@asmeurer
Copy link
Contributor

Yes, @oscarbenjamin explained it well in sympy/sympy#19568 (comment). The problem is that there can be a difference between a dotted name as an attribute lookup and a dotted name as an import, and when there are objects with the same names as modules, there's no obvious way for Sphinx to disambiguate.

Sphinx could maybe try to be smart and try the different ways of viewing a dotted expression and if there is exactly one match, chose that without any errors. I don't know if that is something that would be considered too magic or not. The colon notation seems like it should be sufficient to disambiguate when it is required.

@tk0miya
Copy link
Member

tk0miya commented Jun 25, 2020

I added documentation for :: notation. So I'm closing this now.
Thanks,

@tk0miya tk0miya closed this as completed Jun 25, 2020
tk0miya added a commit to tk0miya/sphinx that referenced this issue Jun 28, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants