sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.46k stars 2.1k forks source link

Sphinx v3.1.1 causing errors for autodocumenting due to import confusion #7844

Closed matthewfeickert closed 4 years ago

matthewfeickert commented 4 years ago

Describe the bug

The pyhf docs build without errors with Sphinx v3.0.4 but they are broken by Sphinx v3.1.1. Specifically, the docs are breaking due to autodocumenting fialing to import the right module, and hence failing to produce the documentation for two sections of the API. This is shown during the build by

WARNING: don't know which module to import for autodocumenting 'optimize.opt_jax.jax_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_minuit.minuit_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_pytorch.pytorch_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_scipy.scipy_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_tflow.tflow_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'tensor.jax_backend.jax_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'tensor.numpy_backend.numpy_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'tensor.pytorch_backend.pytorch_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'tensor.tensorflow_backend.tensorflow_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)

To Reproduce

Steps to reproduce the behavior:

# In a fresh Python 3.7 virtual environment after running
# python -m pip --upgrade pip setuptools wheel

$ git clone https://github.com/scikit-hep/pyhf.git
$ cd pyhf
$ python -m pip install .[docs]
$ cd docs
$ make html # errors during build

Expected behavior

The above commands build without error for Sphinx v3.0.4, so the same behavior is expected for the minor and patch release bump to Sphinx v3.1.1.

Your project

Our project is pyhf and our nightly CI build of the docs catches this.

Environment info

Our CI that runs the docs is using GitHub Actions with the following YAML

  docs:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        python-version: [3.7]
    steps:
    - uses: actions/checkout@v2
      with:
        fetch-depth: 0
    - name: Set up Python 3.7
      uses: actions/setup-python@v2
      with:
        python-version: ${{ matrix.python-version }}

which results in

Additional context This is also pyhf Issue 897

cc @lukasheinrich @kratsg

I'm also not sure if this has any overlap with Sphinx Issue #7841 or not.

tk0miya commented 4 years ago

Note: reproduced with this dockerfile:

FROM python:3.7-slim

RUN apt update; apt install -y git make build-essential vim
RUN git clone https://github.com/scikit-hep/pyhf
WORKDIR /pyhf
RUN pip install -e .[docs]
WORKDIR /pyhf/docs
RUN pip install Sphinx==3.1.1
RUN apt install -y pandoc rsync
RUN make html SPHINXOPTS=-WT
tk0miya commented 4 years ago

I found #7815 should be reimplemented again to build pyhf correctly. It is very difficult to support module and objects are the same name...

matthewfeickert commented 4 years ago

It is very difficult to support module and objects are the same name...

Okay, I guess this means that on our side we should look at restructure/changes then to use modern versions of Sphinx, yes?

kratsg commented 4 years ago

I found #7815 should be reimplemented again to build pyhf correctly. It is very difficult to support module and objects are the same name...

@tk0miya

WARNING: don't know which module to import for autodocumenting 'optimize.opt_jax.jax_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_minuit.minuit_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_pytorch.pytorch_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_scipy.scipy_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'optimize.opt_tflow.tflow_optimizer' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)

while some do have the same names. I'm not sure what you mean by "object" here though.

WARNING: don't know which module to import for autodocumenting 'tensor.jax_backend.jax_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'tensor.numpy_backend.numpy_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'tensor.pytorch_backend.pytorch_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: don't know which module to import for autodocumenting 'tensor.tensorflow_backend.tensorflow_backend' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
michalk8 commented 4 years ago

Yesterday, I took a a closer look a this in our case. Our api.rst contains stuff like this:

.. module:: cellrank.pl
.. currentmodule:: cellrank

.. autosummary::

    :toctree: api/pl
     pl.heatmap
     ...

where .pl is an attribute (abbreviation for plotting module). We can therefore import cellrank.plotting, but not cellrank.pl. However, the line here tries to import cellrank.pl, resulting in ModuleNotFoundError, which is captured by ImportError, which splits the name into 'cellrank' and 'pl.<some_function>' and therefore isn't found. Changong pl.heatmap to plotting.heatmap resolves the issue for us.

@tk0miya I came up with a fix, but I'd say it's onl y a temporary solution.

from sphinx.util import inspect

    parts = name.split('.')
    module = None

    for i, part in enumerate(parts, 1):
        try:
            modname = ".".join(parts[:i])
            try:
                module = import_module(modname)
            except ModuleNotFoundError:
                module = inspect.safe_getattr(module, parts[i - 1])

            # check the module has a member named as attrname
            #
            # Note: This is needed to detect the attribute having the same name
            #       as the module.
            #       ref: https://github.com/sphinx-doc/sphinx/issues/7812
            attrname = parts[i]
            if hasattr(module, attrname):
                value = inspect.safe_getattr(module, attrname)
                if not inspect.ismodule(value):
                    return ".".join(parts[:i]), ".".join(parts[i:])
        except (AttributeError, ImportError):
            return ".".join(parts[:i - 1]), ".".join(parts[i - 1:])
        except IndexError:
            pass

    return name, ""
tk0miya commented 4 years ago

Okay, I guess this means that on our side we should look at restructure/changes then to use modern versions of Sphinx, yes?

I'll try to fix Sphinx again. So please wait a moment. Indeed, it's better not to give the same name to both a module and an object to generate a document by Sphinx. But I'd like to find a workaround to do that. Because the structure would be sometimes needed to keep compatibility and so on.

tk0miya commented 4 years ago

I'm not sure what you mean by "object" here though.

An instance, a function, a class and so on. For example, the name pyhf.optmize has two meaning. One is a package named pyhf.optmize, and another is a pyhf.optimize._OptimizerRetriever object under pyhf module.

>>> import pyhf.optimize
>>> pyhf.optimize
<pyhf.optimize._OptimizerRetriever object at 0x7f570a246590>
>>> import importlib
>>> importlib.import_module('pyhf.optimize')
<module 'pyhf.optimize' from '/pyhf/src/pyhf/optimize/__init__.py'>

For now, autodoc expect one name is matched to a module or an object, not both. I know pyhf is valid python program. So it is better if autodoc will be able to support it.

tk0miya commented 4 years ago

@michalk8

Changong pl.heatmap to plotting.heatmap resolves the issue for us.

I guess your module does not work expectedly if I do from cellrank.pl import heatmap. I thought cellrank.pl is a module-like object, not a module. I understand it might be useful for shortcuts. But it is difficult to detect non-importable module as a module automatically. We need to give a hint to autodoc in some way.

kratsg commented 4 years ago

We do have plans to migrate the retriever code into python3-supported module-level getattr calls. This was only done this way primarily because we needed to support python2 at the time that was written.

On Thu, Jun 25, 2020 at 13:02 Takeshi KOMIYA notifications@github.com wrote:

@michalk8 https://github.com/michalk8

Changong pl.heatmap to plotting.heatmap resolves the issue for us.

I guess your module does not work expectedly if I do from cellrank.pl import heatmap. I thought cellrank.pl is a module-like object, not a module. I understand it might be useful for shortcuts. But it is difficult to detect non-importable module as a module automatically. We need to give a hint to autodoc in some way.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sphinx-doc/sphinx/issues/7844#issuecomment-649704259, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFZ5C76ACC3JSS23FQZQRLRYN7JNANCNFSM4OABMGKA .

-- Dr. Giordon Stark (pronouns: he/him) ATLAS Post-doctoral scholar employee at SCIPP/UCSC https://giordonstark.com/ https://giordonstark.com/?utm_source=kratsg@gmail.com&utm_medium=email_signature

tk0miya commented 4 years ago

Now I confirmed the HEAD of 3.1.x branch can build pyhf's document with no errors.

FROM python:3.7-slim

RUN apt update; apt install -y git make build-essential vim
RUN git clone https://github.com/scikit-hep/pyhf
WORKDIR /pyhf
RUN pip install -e .[docs]
WORKDIR /pyhf/docs
RUN apt install -y pandoc rsync
RUN pip install git+https://github.com/sphinx-doc/sphinx@3.1.x
RUN make html SPHINXOPTS=-WT

I confirmed also sympy's.

FROM python:3.7-slim

RUN apt update; apt install -y git make build-essential vim
RUN git clone https://github.com/sympy/sympy.git
WORKDIR /sympy
RUN pip install mpmath
RUN pip install -e .
WORKDIR /sympy/doc
RUN apt install -y librsvg2-bin imagemagick graphviz
RUN pip install sphinx_math_dollar matplotlib
RUN pip install git+https://github.com/sphinx-doc/sphinx@3.1.x
RUN make html

Finally I reverted some features from 3.1. But it allows to build these project. New package will be bumped soon. Thanks,

matthewfeickert commented 4 years ago

Now I confirmed the HEAD of 3.1.x branch can build pyhf's document with no errors.

Wow. Thank you so much for the hard work that you put in on this @tk0miya! This is really great and we very much appreciate it. :bow: