Reexported classes aren’t linked

[flying-sheep]

For example:

def foo(m: scipy.sparse.spmatrix):
   pass

Will not get a linked parameter type, since the qualname is scipy.sparse.base.spmatrix, and :class:`~scipy.sparse.spmatrix` will therefore point nowhere in the docs index.

[agronholm]

I'm aware of this, but how do you expect me to fix this?

[flying-sheep]

Good question. I’d say, that depends on the reaction to sphinx-doc/sphinx#4826. If they do this quickly, we don’t need to do anything. but if they don’t, maybe we’ll come up with an idea…

Naively, all module hierarchies in the intersphinx mappings could be walked, and a graph constructed for the different locations objects reside in (e.g. walking scipy will reveal that both scipy.sparse and scipy.sparse.base contain the same class). Then we could search all alternative locations in the mapping, and create a substitution dictionary from non-existing doc entries to existing ones. (In our example it would then contain {…, 'scipy.sparse.base.spmatrix': 'scipy.sparse.spmatrix', …})

In pseudocode:

def fullname(modname, obj):
    return f'{modname}.{obj.__qualname__}'

modules = set()
for inv in intersphinx_inventories:
    for name in inv.obj_names():
        if '.' in name:
            modules.add(name[:name.index('.')])

substitutions = {}
for mod_name in modules:
    module = imp.import(mod_name)
    all_funcs_classes = {}
    for object, submodule_name in walk_module(module):
        if isinstance(object, type) or is_function(object):
            all_funcs_classes.setdefault(object, []).append(submodule_name)
    for object, sub_names in all_funcs_classes.items():
        doc_name = None
        for sub_name in set(sub_names):
            qual_name = fullname(sub_name, obj)
            if any(qual_name in inv for inv in intersphinx_inventories):
                doc_name = qual_name
                break
        if doc_name:
            for alias in {fullname(sn, obj) for sn in sub_names} - {doc_name}:
                substitutions[alias] = doc_name

[goerz]

In the absence of sphinx-doc/sphinx#4826 being solved, maybe the autodoc-typehints plugin could support an option in conf.py that maps qualnames to canonical names. For example, in reaction to

WARNING: py:class reference target not found: qutip.qobj.Qobj

in https://readthedocs.org/projects/weylchamber/builds/8294327/, I could manually define autodoc_typehints_name_map = {'qutip.qobj.Qobj': 'qutip.Qobj'} in conf.py. It's a crude workaround, but it's better than nothing. To be honest, problems like sphinx-doc/sphinx#4826 in the basic tooling is an indication to me that type-annotations are nowhere near ready for prime-time.

[flying-sheep]

As a crude hack to do this in your own project now, you can put the following in your conf.py

(this is implemented in scanpydoc.elegant_typehints)

import sphinx_autodoc_typehints

qualname_overrides = {
    'anndata.base.AnnData': 'anndata.AnnData',
    'scipy.sparse.base.spmatrix': 'scipy.sparse.spmatrix',
    'scipy.sparse.csr.csr_matrix': 'scipy.sparse.csr_matrix',
    'scipy.sparse.csc.csc_matrix': 'scipy.sparse.csc_matrix',
}

fa_orig = sphinx_autodoc_typehints.format_annotation
def format_annotation(annotation):
    if inspect.isclass(annotation):
        full_name = f'{annotation.__module__}.{annotation.__qualname__}'
        override = qualname_overrides.get(full_name)
        if override is not None:
            return f':py:class:`~{override}`'
    return fa_orig(annotation)
sphinx_autodoc_typehints.format_annotation = format_annotation

[agronholm]

That problem has nothing to do with type annotations. I was struggling with it way before type annotations were introduced to Python.

[flying-sheep]

Of course, the problem surfaces whenever you programmatically access a reexported class’ qualified name and expect to link to its docs. This project just happens to work that way and therefore exposes the problem.

[agronholm]

One possible solution is to do what trio does: change the qualified name of the classes and functions: https://github.com/python-trio/trio/blob/master/trio/__init__.py

[flying-sheep]

Hmm, that only works if we know which the canonical module is right?

Or do you mean that the original modules could do that? Then we have to convince them to do it.

Of course we could also do what I said in https://github.com/agronholm/sphinx-autodoc-typehints/issues/38#issuecomment-379756476. Or we could just try to import all the names in intersphinx_mapping and whenever we encounter a class in format_annotation, we could try to find it in the list of canonical names:

canonical_paths = {}
for url, altfile in intersphinx_mapping.values():
    inv = fetch_inventory(..., url, altfile)
    for sect_name, section in inv.values():
        if not sect_name.startswith('py:'): continue
        for item_path in section.keys():
            o = try_import(item_path)
            if o is not None:
                canonical_paths[id(o)] = item_path

def format_annotation(annotation):
    ...
    if inspect.isclass(annotation):
        full_name = canonical_paths.get(
            id(annotation),
            f'{annotation.__module__}.{annotation.__qualname__}',
        )
        ...
    ...

actually I think that’s very doable!

[goerz]

That's pretty much exactly what I had in mind! Thanks!

…

As a crude hack to do this in your own project now, you can put the following in your conf.py> import sphinx_autodoc_typehints qualname_overrides = { 'anndata.base.AnnData': 'anndata.AnnData', 'scipy.sparse.base.spmatrix': 'scipy.sparse.spmatrix', 'scipy.sparse.csr.csr_matrix': 'scipy.sparse.csr_matrix', 'scipy.sparse.csc.csc_matrix': 'scipy.sparse.csc_matrix', } fa_orig = sphinx_autodoc_typehints.format_annotation def format_annotation(annotation): if inspect.isclass(annotation): full_name = f'{annotation.__module__}.{annotation.__qualname__}' override = qualname_overrides.get(full_name) if override is not None: return f':py:class:`~{override}`' return fa_orig(annotation) sphinx_autodoc_typehints.format_annotation = format_annotation> — You are receiving this because you commented.

[goerz]

That problem has nothing to do with type annotations. I was struggling with it way before type annotations were introduced to Python.
I suppose you're right.

It's just that when I do the documentation manually, I will write

Args:
    m (scipy.sparse.spmatrix): sparse matrix parameter

in my docstring, somehow having figured out that that's what generates a working link the rendered Sphinx docs. So you mean that before type annotations, the problem was manually figuring out the canonical name for the linking, right?

With type annotations, it seems I just don't have that option. There, I will have

from scipy.sparse import spmatrix  # or, `from scipy.sparse.base import spmatrix`

def f(m: spmatrix):
    """My function

    Args:
        m: sparse matrix parameter
    """
    pass

with no control over what link is generated. In theory, the type annotated version is nicer, because those manual type specifications in the docstring tend to take up a lot of space. But of course, the point is moot unless Sphinx links correctly by figuring out the canonical name automatically.

I actually don't think I fully understand how scipy manages to define spmatrix in scipy.sparse.base, but the canonical link location is in scipy.sparse, where it is re-exported.

I always thought it would be really nice if Sphinx in general allowed me to specify the type with any importable specification, and it would link correctly. In my own projects, I generally have classes defined in deep submodules, but they are always re-exported in the main module. It would be much nicer if in all of my docstrings I could write

Args:
    a (mypkg.mycls): a parameter of type :class:`~mypkg.mycls`

instead of

Args:
    a (mypkg.sub1.sub2.mycls): a parameter of type :class:`~mypkg.sub1.sub2.mycls`

and the link would still go to mypkg.sub1.sub2.mycls. But I digress...

[goerz]

I suppose that's more or less what sphinx-doc/sphinx#4826 is all about

[agronholm]

I will try to find a workaround. I too have significant problems with this in my own projects.

[flying-sheep]

@agronholm: what do you think about the solution outlined at the bottom of https://github.com/agronholm/sphinx-autodoc-typehints/issues/38#issuecomment-448552479?

[agronholm]

I think it would be better to just add a hook for a callable that looks up the canonical path. Or did I miss something?

[flying-sheep]

My solution would work automatically and without user interaction.

[agronholm]

My solution would work automatically and without user interaction.

Could you explain how it works? Due to the pseudocode nature it's not that easy to follow.

[goerz]

As far as I can tell: For every object in any intersphinx mapping, it imports that object and stores a map from the actual object (via its id) to the canonical reference-URL. Then, whenever it sees an annotation (that is an actual imported object), it looks up its reference-URL in the dict created in step 1.

Seems like a good solution to me. Actually, I think it's exactly the behavior that sphinx-autodoc-typehints should have, independent of what Sphinx and others do to fix related problems.

[flying-sheep]

Yeah, I’m also pleased with the idea. And you got it right, except for

a map from the actual object (via its id) to the canonical reference-URL

It’s a map from the object ID (correct) to the object path (f'{o.__module__}.{o.__qualname__}'), not an URL. I’ll comment it:

# create a map of object IDs to paths (the.module.path.TheQualName)
canonical_paths = {}
for url, altfile in intersphinx_mapping.values():
    # the intersphinx mapping contains urls and alternative filenames
    # (if the file name differs from `objects.inv`).
    # `fetch_inventory` downloads, caches and parses that file
    inv = fetch_inventory(..., url, altfile)
    for sect_name, section in inv.values():
        # the objects.inv contains sections named e.g.
        # py:method for python objects, but also
        # other stuff we’re not interested in (e.g. C functions)
        if not sect_name.startswith('py:'): continue
        for item_path in section.keys():
            # This tries to import an object by its full name.
            # Not entirely easy due to qualnames
            o = try_import(item_path)
            if o is not None:
                canonical_paths[id(o)] = item_path

# this is the function in the main lib responsible for finding the
# type annotation for any living object and generating the
# reStructuredText to render hyperlinks to types.
def format_annotation(annotation):
    ...
    if inspect.isclass(annotation):
        # we try first to get the canonical path from the dict we built before
        # and if we fail, we use the object’s full name
        full_name = canonical_paths.get(
            id(annotation),
            f'{annotation.__module__}.{annotation.__qualname__}',
        )
        ...
    ...

[agronholm]

I have not yet found any way to access the inventory from the hooks, but if that were possible, I could check if the object exists and if not, check higher levels for an object with the same type and name which would fix most reexports.

[flying-sheep]

Of course it’s possible:

• env.intersphinx_inventory contains {'py:function': {'qual.name': (mod_name, version, url, title), ...}, ...}
• env.intersphinx_named_inventory contains dict(mapping_name={'py:function': {...}, ...}, ...)

Actually astropy has code that does it.

[agronholm]

Thanks, I will look at this later and see if it can help to fix the issue.

[agronholm]

But does that exist even when intersphinx is not enabled, and does it contain the inventory for the current project?

[flying-sheep]

No and no. They manually create an inventory for the current project (env.class_name_mapping) from the autodoc-process-docstring handler and consult both the intersphinx inventory and that one in the missing-reference handler. I think doing the exact same thing might be a good idea.

[agronholm]

I think I'm still missing a piece of the picture. How does the missing-reference hook help us figure out the correct reference?

[agronholm]

Ok, I think I might get it now – the autodoc-process-docstring handler gets both the API name and the actual object, and this could be used to fix the problem. I'll post an update once I've experimented a bit.

[agronholm]

Ok, I think I'm making some headway with this. Next I need to construct two tests:

1. A reference to a class in the current project re-exported to the top level package
2. A reference to a class from the standard library that has been re-exported to another package (e.g. concurrent.futures.ThreadPoolExecutor)

[flying-sheep]

Hi, if you’re stuck, how about publishing the branch with your partial work so one of us can finish it?

[agronholm]

I had totally forgotten about this. I'll see if I can find the code.

[agronholm]

I can't find anything relevant in local branches or the git shelf. Seems like I didn't get anywhere yet regarding code updates.

[eric-wieser]

One possible solution is to do what trio does: change the qualified name of the classes and functions: https://github.com/python-trio/trio/blob/master/trio/__init__.py

Doing this results in inspect.getsource(MyClass) failing, which in turn means that :member-order: bysource fails in sphinx.

[letmaik]

Just a note for others, the following steps made it work automatically for me:

1. Copy smart_resolver.py in doc/ext.
2. In conf.py, add sys.path.append(os.path.join(os.path.dirname(__file__), 'ext')) and modify the extensions like so:

extensions = [
  ...,
  'sphinx.ext.intersphinx',
  'sphinx_autodoc_typehints',
  'smart_resolver'
  ]

[auscompgeek]

Instead of vendoring that module as suggested above, I would encourage people to pull in sphinx-automodapi as a dependency. No path hacks necessary.

[letmaik]

Good call, I wasn't aware that it supports smart_resolver to be consumed separately:

extensions = [
...
'sphinx_automodapi.smart_resolver'
]

Shame that this isn't part of sphinx or some official sphinx extension.

[flying-sheep]

As of now, it is: sphinx-doc/sphinx#9026

[gaborbernat]

Resolved by the linked upstream change.