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

Reference target not found due to import aliases #10198

Closed
adamjstewart opened this issue Feb 15, 2022 · 8 comments
Closed

Reference target not found due to import aliases #10198

adamjstewart opened this issue Feb 15, 2022 · 8 comments

Comments

@adamjstewart
Copy link
Contributor

Describe the bug

See #9395 for context. In #9395 we determined that hacking __module__ attributes of classes isn't necessary when using Sphinx 4. I tried removing our hacks in microsoft/torchgeo#401 but now I'm encountering a build issue: https://readthedocs.org/projects/torchgeo/builds/16098659/

Basically, source code looks like:

class Foo:
    #: thing that does stuff
    thing = 0

    #: adds on to other :attr:`thing`
    stuff = 0

The error message Sphinx gives is:

WARNING: py:attr reference target not found: thing

Oddly enough, it seems to locate one of the other attributes without any issues.

How to Reproduce

$ git clone https://github.com/microsoft/torchgeo.git
$ cd torchgeo
$ git switch docs/__module__
$ pip install .
$ cd docs
$ pip install -r requirements.txt
$ make html

You should see a build error like:

docstring of torchgeo.datasets.RasterDataset.filename_regex:8: WARNING: py:attr reference target not found: separate_files`

Expected behavior

I would expect this to work without having to hack __module__ since I'm using Sphinx 4.

Your project

https://readthedocs.org/projects/torchgeo

Screenshots

No response

OS

Linux

Python version

3.7

Sphinx version

4.4.0

Sphinx extensions

sphinx.ext.autodoc

Extra tools

No response

Additional context

Follow-up to discussion in #9395 @tk0miya

@adamjstewart
Copy link
Contributor Author

@tk0miya any thoughts on this? I finally convinced PyTorch/torchvision to upgrade from Sphinx 3 to 5 in hopes of fixing these kinds of import alias issues but I'm finding that this didn't actually change anything. See microsoft/torchgeo#637 for the PR where I try to remove the hacks I added to __module__ (as well as links to the background on this bug). From what you said in #9395 it sounds like these __module__ hacks should not be necessary in Sphinx 4+, but I'm finding that they are still necessary.

@AA-Turner AA-Turner added this to the 5.x milestone Jun 30, 2022
@AA-Turner
Copy link
Member

Is there a more minimal reproducer than the entire TorchGeo documentation?

A

@adamjstewart
Copy link
Contributor Author

@AA-Turner here is the smallest possible minimal reproducer I can come up with.

  1. Create an example.py file containing:
    import torch.nn as nn
    
    class MyNeuralNetwork(nn.Module):
        pass
  2. Run sphinx-quickstart to get a default conf.py, index.rst, and Makefile
  3. Add the following lines to index.rst:
    .. module:: example
    
    .. autoclass:: MyNeuralNetwork
       :show-inheritance:
  4. Add the following lines to conf.py:
    extensions = [
        "sphinx.ext.autodoc",
        "sphinx.ext.intersphinx",
    ]
    
    intersphinx_mapping = {
        "torch": ("https://pytorch.org/docs/stable", None),
    }
    
    nitpicky = True
  5. Run make html and you should observe the following error:
    example.py:docstring of example.MyNeuralNetwork:1: WARNING: py:class reference target not found: torch.nn.modules.module.Module
    

You should be able to reproduce this with any class or function that uses import aliases. In the case of PyTorch, the Module class is defined in torch/nn/modules/module.py, but torch/nn/modules/__init__.py uses:

from .module import Module

and torch/nn/__init__.py uses:

from .modules import *

This results in the Module class being documented under torch.nn in the documentation but Module.__module__ instead points to torch.nn.modules.module.Module.

Let me know if you want a minimal reproducer that doesn't rely on an external library (PyTorch is also massive). It should be possible to see the same problem within a single library as long as you use import aliases.

@tk0miya
Copy link
Member

tk0miya commented Jul 3, 2022

Indeed, the objects.inv of pytorch does not contain torch.nn.modules.module.Module.

$ python -m sphinx.ext.intersphinx https://pytorch.org/docs/stable/objects.inv | grep torch.nn.modules.module.Module
$

It seems they're using Sphinx-5.0. So it's strange to me.
https://github.com/pytorch/pytorch/blob/caa6ef15a294c96fad3bf67a10a8b4fa605080bb/docs/requirements.txt#L1

We need to investigate why it does not contain the canonical name of the class...

@tk0miya
Copy link
Member

tk0miya commented Jul 3, 2022

Let me know if you want a minimal reproducer that doesn't rely on an external library (PyTorch is also massive). It should be possible to see the same problem within a single library as long as you use import aliases.

It's very helpful. Could you make it if you have time?

@adamjstewart
Copy link
Contributor Author

I'm having trouble reproducing this with only a single repo. I think it will require at least 2 repos. The originally issue I reported in the first comment of this issue (WARNING: py:attr reference target not found: separate_files) is actually different than the issue I reported later (WARNING: py:class reference target not found: torch.nn.modules.module.Module). In the former, separate_files isn't being rendered in the documentation despite #: comments above the variable. In the latter, the class is in the docs, but Sphinx can't find the correct location. Should I split this issue into two separate issues? Or should we try to track them down one at a time in this issue?

@AA-Turner
Copy link
Member

Or should we try to track them down one at a time in this issue?

My intuition would be that they're closely related, so for now lets just keep this one issue. If we find out that there are two entirely different causes, we can split later.

A

@adamjstewart
Copy link
Contributor Author

Found a workaround for this. Simply replace:

    #: adds on to other :attr:`thing`

with:

    #: adds on to other :attr:`~Foo.thing`

I'm satisfied with this workaround, so I think we can close this. Thanks for the help!

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