sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

more than one target found for cross reference should prefer current file's method over other libraries methods

Open dvorapa opened this issue 7 years ago • 17 comments

Subject: more than one target found for cross reference should prefer current file's method over other libraries methods

Problem

  • When building HTML of our project, Sphinx outputs cca 200 warnings like this. But half of them should be never thrown, because Sphinx is not sure, whether to use current file's one or imported library's one. For example, see:
/src/pywikibot/interwiki_graph.py:docstring of pywikibot.interwiki_graph.GraphDrawer.__init__:: WARNING: more than one target found for cross-reference 'Subject': scripts.interwiki.Subject, pywikibot.interwiki_graph.Subject

The expected one is clear: The one from current file interwiki_graph should be preferred over the imported, but Sphinx still throwns a warning.

Procedure to reproduce the problem

Build HTML of Pywikibot (last build by us: https://integration.wikimedia.org/ci/job/pywikibot-core-tox-publish/424/console) or try to simulate the situation

Error logs / results

/src/pywikibot/interwiki_graph.py:docstring of pywikibot.interwiki_graph.GraphDrawer.__init__:: WARNING: more than one target found for cross-reference 'Subject': scripts.interwiki.Subject, pywikibot.interwiki_graph.Subject
  • https://integration.wikimedia.org/ci/job/pywikibot-core-tox-publish/424/console

Expected results

It should prefer the current file over the imported one

Reproducible project / your project

  • https://www.mediawiki.org/wiki/Manual:Pywikibot

Environment info

  • OS: Debian Jessie
  • Python version: 3.4
  • Sphinx version: 1.7.4
  • Napoleon, Epytext

dvorapa avatar May 12 '18 09:05 dvorapa

Where can I see the content ofinterwiki_graph? I'd like to see your source and try to reproduce this. I can't understand how to do it. Please let me know the way to reproduce (or investigate).

Reproducible project / your project https://www.mediawiki.org/wiki/Manual:Pywikibot

tk0miya avatar May 12 '18 16:05 tk0miya

Reproducible project / your project https://www.mediawiki.org/wiki/Manual:Pywikibot

As you could find out in that link, the source code can be found for example here: https://phabricator.wikimedia.org/diffusion/PWBC/. Also you could look to https://pypi.org/project/pywikibot/.

Sphinx config is in the docs/ folder. Dependencies for Sphinx are written in docs/conf.py and tox.ini's [testenv:doc] (or I could collect them into one list and send them to you, if there will be any problem). The Sphinx build live can be found here: https://doc.wikimedia.org/pywikibot/

The file in the description is stored in pywikibot/ folder. But if you search through build log, you could see there is also a bunch of other similar ones. Today, I wanted to clean "more than one target found for cross reference" warnings, but I found out many of them have this issue.

If there will be any problem, I'll write more detailed step-by-step quide to build our docs.

dvorapa avatar May 12 '18 23:05 dvorapa

Confirmed with this Dockerfile:

FROM tk0miya/sphinx-html

RUN apt update; apt install -y libmysqlclient-dev
RUN git clone https://gerrit.wikimedia.org/r/pywikibot/core.git
WORKDIR /docs/core
RUN git submodule update --init
RUN pip3 install tox
RUN sed -i -e 's/python3.4/python3.5/' tox.ini
RUN tox -e doc

I understand the issue.

But half of them should be never thrown, because Sphinx is not sure, whether to use current file's one or imported library's one.

At present, there are no way to tell the objects in the file to Sphinx. The autodoc extension only generates reST file. And Sphinx-core only converts it to output format.

I feel your proposal is reasonable. So it would be nice if we can implement it in future!

tk0miya avatar May 13 '18 14:05 tk0miya

hi there! i'm curious about this thread as i'm running into a similar issue with a pr for nbconvert . what is weird is that it's throwing warnings only in circleCI not in my local build and also on pages that are just text but have api content below such as this page. and it's not throwing warnings (treated as errors) each run, it throws ONE of them and quits. then i fix it and another pops up. Does anyone have a suggestion regarding how to treat these warnings by chance? @tk0miya it would be wonderful to have a clean CI build.

lwasser avatar May 18 '21 14:05 lwasser

Is there any workaround at the moment? I have classes with the same name in different modules in my project and I get the error (removed acutal names):

Warning, treated as error:
/opt/.../client.py:docstring of BarBaz.function::more than one target found for cross-reference 'ClassA': module_x.ClassA, module_z.ClassA

Am I forced to rename the classes now or is there a way to ignore this particular situation?

mezhaka avatar Jun 04 '21 16:06 mezhaka

On my project, the doc builds correctly for Sphinx 3.x and fails (because warnings treated as errors) on 4.x. I'll need to stay on 3.x until it's resolved, but if you have an idea where to look for, I can try poking and see if I find a fix :)

ewjoachim avatar Jul 29 '21 20:07 ewjoachim

This bug out of no where has started impacting us from yesterday https://github.com/abhinavsingh/proxy.py/runs/4836507299?check_suite_focus=true

It all started after we add a file named pool.py in a separate module. A file name pool.py already exists in another module. But Sphinx cannot figure this out leading into warnings and failed CI.

abhinavsingh avatar Jan 17 '22 05:01 abhinavsingh

There's a similar but different issue causing the same messages at #10088 and an ongoing PR in #10089 . Is there a chance it solves your issue ?

ewjoachim avatar Jan 17 '22 09:01 ewjoachim

@ewjoachim Yep, that worked. When can we expect the next release (or pre-release). We'll need to remove the diff below at some point.

diff --git a/docs/requirements.in b/docs/requirements.in
index 24f2e03..263030d 100644
--- a/docs/requirements.in
+++ b/docs/requirements.in
@@ -1,6 +1,6 @@
 myst-parser[linkify] >= 0.15.2
 setuptools-scm >= 6.3.2
-Sphinx >= 4.3.0
+Sphinx @ git+https://github.com/ewjoachim/sphinx@py-domain-canonical-any-10088
 furo >= 2021.11.15
 sphinxcontrib-apidoc >= 0.3.0
 sphinxcontrib-towncrier >= 0.2.0a0

abhinavsingh avatar Jan 17 '22 17:01 abhinavsingh

There's a similar but different issue causing the same messages at #10088 and an ongoing PR in #10089 . Is there a chance it solves your issue ?

Sorry bad news is that CI still failed , ref https://github.com/abhinavsingh/proxy.py/runs/4844619931?check_suite_focus=true

Unfortunately, I am not sure about our setup either. There is a .in and .txt requirements file. I have consulted the contributor who added this workflow and waiting upon them.

abhinavsingh avatar Jan 17 '22 18:01 abhinavsingh

Update:

Looks like we cannot try the patch out until release. Other CI runs starts to throw unable to verify hashes from private repo. Also, I am unsure if the patch completely fixes the issue.

Latest run https://github.com/abhinavsingh/proxy.py/runs/4845237618?check_suite_focus=true

We are also seeing newer could be replaced by an extlink (try using ':issue:`642#issuecomment-960819271`' instead) errors. All in all, we have 97 new warnings :(

Screen Shot 2022-01-18 at 1 01 57 AM

abhinavsingh avatar Jan 17 '22 19:01 abhinavsingh

Before Sphinx had a better solution for resolving type annotations I created a package called sphinx-resolve-py-references that did some static analysis to disambiguate what was being referred to. It basically traces the imports back to the original definition. The core of the Sphinx related logic can be found here and the static analysis via the AST can be found here. Perhaps this is something that could be contributed to the core of sphinx?

Depending on how it's implemented it would also mean that you could reference classes imported in the same module without having specify the full path:

from some_package import MyClass

def my_function():
    '''Uses :class:`MyClass`'''
    # The above reference would normally fail because `MyClass`
    # is not defined in this module. However this extension
    # automatically resolves the reference

rmorshea avatar Mar 26 '22 20:03 rmorshea

Apparently Sphinx gets really confused about the new annotations in python and assumes that classes are defined in the files that in fact are only using them as types.

from __future__ import annotations
from foo import Bar

x: Bar  # <-- Guess what, now sphinx believes that Bar is defined in this file too

ssbarnea avatar Aug 24 '22 17:08 ssbarnea

Might be related or not, but https://github.com/sphinx-doc/sphinx/pull/10089 was recently merged, and maybe it's involved in the change of behaviour you observed. Let me know: if I as the one breaking things, I'd love to know and try to fix.

ewjoachim avatar Aug 25 '22 13:08 ewjoachim

@ewjoachim Unrelated as I was already using 5.1.1 which included that fix. I found as a workaround to avoid using annotations for these types, like:

from typing import TYPE_CHECKING
if TYPE_CHECKING:
  from foo import Bar

x: "Bar"

UPDATE: While this workaround worked in few cases, i reached others where it does not seem to work.

ssbarnea avatar Sep 21 '22 14:09 ssbarnea

@ssbarnea do you have a consistently failing reproducer?

A

AA-Turner avatar Sep 21 '22 17:09 AA-Turner

Unrelated as I was already using 5.1.1

It's the other way around. My question is whether or not the fix included in 5.1.1 introduced the issue.

Also, you may want to look at from __future__ import annotations :)

ewjoachim avatar Sep 22 '22 13:09 ewjoachim

@rmorshea: Your sphinx-resolve-py-references fork is extremely impressive. Sadly, it's also unmaintained – which is no slight on your breathtaking hard work. I wouldn't want to maintain a non-trivial Sphinx monkey patch in perpetuity, either. :sweat_smile:

Sadly, I incorrectly assumed Sphinx resolved relative references against the current module out-of-the-box and then documented a several thousand-line runtime type-checker under that assumption. Several hundred sphinx-build warnings later, I now realize this is where the venerable saying about assumptions comes from. I briefly contemplated inlining the core logic of @rmorshea's sphinx-resolve-py-references fork into @beartype's conf.py configuration. Then I realized that meant I would need to maintain a non-trivial Sphinx monkey patch in perpetuity. The sweat pouring off my forehead is real, people.

I'm now furiously canonicalizing all relative references into absolute references. Since this is terrible, I find myself pounding the keyboard repeatedly. Is there really no sane solution for this four year-old issue in 2022? This should really just work by default:

from some_package import MyClass

def my_function():
    '''Uses :class:`MyClass`'''

EDIT: We gave up on canonicalizing all relative references into absolute references. Instead, we just ruthlessly silenced all several hundred warnings with a trivial workaround shamelessly pilfered from @RDFLib:

# List of zero or more Sphinx-specific warning categories to be squelched (i.e.,
# suppressed, ignored).
suppress_warnings = [
    #FIXME: *THIS IS TERRIBLE.* Generally speaking, we do want Sphinx to inform
    #us about cross-referencing failures. Remove this hack entirely after Sphinx
    #resolves this open issue:
    #    https://github.com/sphinx-doc/sphinx/issues/4961
    # Squelch mostly ignorable warnings resembling:
    #     WARNING: more than one target found for cross-reference 'TypeHint':
    #     beartype.door._doorcls.TypeHint, beartype.door.TypeHint
    #
    # Sphinx currently emits *HUNDREDS* of these warnings against our
    # documentation. All of these warnings appear to be ignorable. Although we
    # could explicitly squelch *SOME* of these warnings by canonicalizing
    # relative to absolute references in docstrings, Sphinx emits still others
    # of these warnings when parsing PEP-compliant type hints via static
    # analysis. Since those hints are actual hints that *CANNOT* by definition
    # by canonicalized, our only recourse is to squelch warnings altogether.
    'ref.python',
]

That'll do, Sphinx. That'll do.

leycec avatar Oct 06 '22 07:10 leycec

@leycec if there's interest, I'd be willing to update the package. I just merged some minor updates (unreleased) but it's hard to know if I've covered all the edge cases that people may have in their own projects.

rmorshea avatar Oct 08 '22 21:10 rmorshea

@rmorshea would you be interested in merging it to the Sphinx core?

A

AA-Turner avatar Oct 08 '22 21:10 AA-Turner

That would be great. Though, it would be useful if someone more familiar with Sphinx took a look at it to see if all the heuristics I came up with make sense. It's also not the cleanest code I've ever written, so it might require some work on my end to clean things up a bit before that review.

rmorshea avatar Oct 08 '22 23:10 rmorshea

Hi @rmorshea,

I came across this issue as I am affected myself and across your extension. I gave it a try, simply pulled the source into my venv as I wanted to try the latest updates. ;-)

However, as soon as I activate this extension I get additional warnings on my builds while the cross-reference warnings still persist. My new warnings look like this:

WARNING: No referent for name 'mylib.core.base.BaseDataAPI' in 'C:\\<...>\\src\\mylib\\resources\\something\\mymodule.py:docstring of mylib.resources.something.mymodule.MySpecificAPI' via module 'mylib.resources.something'

The question is, what does this mean? My structure:

mylib.resources.backbone.mymodule.MySpecificAPI

  • This is a class in mymodule.py.
  • The docstring contains no references at all.
  • The class is a subclass of BaseDataAPI, and that's the only reference to BaseDataAPI.
  • BaseDataAPI is imported in mymodule.py with from ...core.base import BaseDataAPI.

mylib.core.base.BaseDataAPI

  • This is a class in base.py.
  • Regarding MySpecificAPI there is no more to say - its simply a subclass and its unknown/unrelated from BaseDataAPI class's perspective.

My Env:

  • Python 3.7
  • Sphinx 5.2.3

@AA-Turner Regarding the cross-reference warnings (to share some experience): My usecase is fairly simple. Example with one warning regarding my custom exception InvalidError:

  • Primary access: mylib.exceptions.errors.InvalidError
  • Secondary access: mylib.exceptions.InvalidError, provided via import to simplify public API structure.
  • Referenced in docstring (example): mylib.resources.something.mymodule.MySpecificAPI as relative reference: :raises: InvalidError. It is imported via from ...exceptions import InvalidError.

So this is fairly simple in terms of complexity. My expectation would be, that Sphinx sees this docstring reference, tries to find InvalidError locally in module.py where it is referenced, then looks at the imports and traces it back to its secondary access point. But without warnings. :D Hopefully this will work at least with rmorshea's extension after it's included to the Sphinx core. Right now it's not working in my case.

Kind regards, Martin

martinpakosch avatar Oct 20 '22 08:10 martinpakosch

Hey @martinpakosch, I made an issue in my repository where I'll answer this question. For others who have questions specifically about my extension it seems like it would be best to post an issue there rather than in this thread.

rmorshea avatar Oct 20 '22 09:10 rmorshea

Cool, thx @rmorshea. Was not able to create it directly due to the previous "archive" state of your repo. :D

martinpakosch avatar Oct 20 '22 09:10 martinpakosch

@AA-Turner @rmorshea Any thoughts on progressing a merge of this code?

ShaheedHaque avatar Mar 17 '23 11:03 ShaheedHaque

What solved this issue for me is setting the following autodoc option in conf.py:

autodoc_default_options = {
    'ignore-module-all': True
}

https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#directive-option-automodule-members

sigma67 avatar May 11 '23 11:05 sigma67