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 v3.1.1 causing errors for autodocumenting due to import confusion #7844

Closed
matthewfeickert opened this issue Jun 16, 2020 · 11 comments
Closed

Comments

@matthewfeickert
Copy link

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
Copy link
Member

tk0miya commented Jun 21, 2020

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
Copy link
Member

tk0miya commented Jun 21, 2020

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
Copy link
Author

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
Copy link

kratsg commented Jun 23, 2020

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
Copy link

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
Copy link
Member

tk0miya commented Jun 25, 2020

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
Copy link
Member

tk0miya commented Jun 25, 2020

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
Copy link
Member

tk0miya commented Jun 25, 2020

@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.

tk0miya added a commit that referenced this issue Jun 25, 2020
Fix #7844: autodoc: Failed to detect module when relative module name given
@kratsg
Copy link

kratsg commented Jun 25, 2020 via email

@tk0miya
Copy link
Member

tk0miya commented Jul 5, 2020

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
Copy link
Author

matthewfeickert commented Jul 5, 2020

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. 🙇

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 22, 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

4 participants