Don't warn on missing inventory in import statement if import target resolves

[DanCardin]

Issue

Example given below. Generally, it seems that many modules or packages will erroneously warn, presumably when they dont have dedicated pages.

In the below example, sqlalchemy.ext.declarative resolves and is clickable, as it brings you to the correctly annotated page.

Versus from sqlalchemy.orm import declarative_base where sqlalchemy.orm has no corresponding page and warngs, but declarative_base resolves correctly.

Expected behavior

Perhaps for successfully resolved imported items, the package or module in which it resides should not warn if there's no corresponding link for it.

That still leaves import sqlalchemy. Naively i might expect that to link to the root of the docs if there's otherwise no resolved link for it.

Steps to reproduce

Although, and this may still be user error, I'm seeing:

docs/source/experimental_tests.rst:69: WARNING: Inventory missing `sqlalchemy` when resolving `sqlalchemy` on line 1
docs/source/experimental_tests.rst:69: WARNING: Inventory missing `sqlalchemy` when resolving `sqlalchemy` on line 2
docs/source/experimental_tests.rst:69: WARNING: Inventory missing `sqlalchemy.ext.declarative.declarative_base` when resolving `declarative_base` on line 3
WARNING: Cannot locate modules: ..., 'sqlalchemy'

for

.. code-block:: python

   import sqlalchemy
   from sqlalchemy import Column, types
   from sqlalchemy.ext.declarative import declarative_base
   from sqlalchemy.orm import declarative_base  # or this

with

intersphinx_mapping = {
    "sqlalchemy": ("https://docs.sqlalchemy.org/en/14", None),
    ...

Column, types, sqlalchemy.ext.declarative, and the 2nd declarative_base are clickable.

[felix-hilden]

Thanks for submitting! I might have misunderstood your details, so I'll try the example later myself. But at a glance everything seems to be working as intended, because we do want to link import statements as well. You didn't expect missing modules to warn as well, and I can see how that would be beneficial here, but I'm quite cautious to start ignoring them because arguably if you've asked for extra warnings you want them all. Similarly, leaving the top-level namespace out of the object inventory is a choice (or omission) by the library's developers.

[DanCardin]

I suppose in the working example https://github.com/sqlalchemy/sqlalchemy/blob/main/doc/build/orm/extensions/declarative/index.rst, perhaps it's .. currentmodule:: sqlalchemy.ext.declarative that makes that particular example work. But that seems like something that'd be less common than not, I'd never seen it before.

Fwiw I'm happy for modules that literally dont exist to warn, but in this case, you're

For example from sphinx_codeautolink import clean_pycon produces similar errors for me because sphinx_codeautolink isn't a linkable target. If sphinx_codeautolink is part of a from which produces a working link for clean_pycon, then it seems to me like it should either not link or (this is potentially prone to not actually working how i imagine) link to the page that the import portion links to, but without the #link-target portion.

It's not like you could omit the from sphinx_codeautolink portion of the import statement, so it basically just yields an unavoidable warning unless the docs-writer happened to include a module-level document, which I imagine is going to not be the case in most cases. And the thing you probably actually wanted to link is the target of the import i.e. clean_pycon. While you might also want the from portion when it's available, if there's nothing to link the warning seems spurious.

[felix-hilden]

.. currentmodule::

Could be, although I only know that it happens on .. automodule::.

from sphinx_codeautolink import clean_pycon

Fair enough, but as a library developer, if I wanted to be careful and make sure that all links work, I'd want to know that the top level namespace isn't referenced anywhere. I'd want to add it along with any submodules as well. And I'd want to structure my library so that the module links make sense and take a user to sensible locations, not just random modules where a class happens to be defined. I feel like the warnings help in that case.

But this doesn't help you, does it 😅 and I think having yet another configuration option for "show me warnings but not quite as much" seems a bit weird. So what's your current situation? Would you consider just checking the warnings once and leaving them off once you've dealt with every case you have control over?

or link to the page that the import portion links to, but without the #link-target portion

I think we can't assume that the module is documented on the same page as its objects.

[DanCardin]

I was hoping to leave these one and set -W error to prevent people (me) from merging docs that have broken example code.

Naively it seems like the logic i think i want is (using vocab that hopefully matches your code and makes sense): If a given LinkContext.import_from does not successfully have an item in its corresponding inventory (happy for it to warn if it's missing inventory entirely, i.e. no intersphinx config) do not warn.

From my perspective, it seems like a progression of increasing strictness (rather than "show me warnings but not quite as much") especially because I can't actually avoid triggering this kind of warning because the from path is not optional. Although, I can appreciate not wanting a complex and hard to understand set of configurations.

[felix-hilden]

That would be a nice use of the option. Btw, look out for sphinx-doc/sphinx#10110 for -W not working quite correctly in our case.

But, I have a slight issue with the proposition that we should use other missing inventory items (or a whole module) as the marker. Firstly, modules may have different names to the libraries that they are shipped with. A common one is scikit-learn which is imported as sklearn. So examining the intersphinx configuration could just lead to more confusion.

Maybe we could have a directive for suppressing warnings in a particular example. I think that would suppress all warnings. But I'll have to think about it. How would that sound? Not a promise though 😄

[DanCardin]

I dont think i'm suggesting using other missing items as a marker.

from sqlalchemy import Column

If Column links to something. Then clearly sqlalchemy is not actually missing (you traversed the sqlalchemy portion of the import to determine to what Column is referencing!), it's just that there's no general sqlalchemy page target, and you're encountering an otherwise impossible to avoid warning.

So i'm just suggesting taking into account whether the right hand side of import in a from ... import ...-style import links, as a mechanism for telling you whether the left-hand-side of the import's failure to import is actually a problem.

[felix-hilden]

Thanks for clarifying. I'm not willing to dilute the value of our extra warnings by default though, so I've opened #104 to consider other ways of suppressing warnings. I still think that any failed inventory access should produce a warning if you've explicitly asked for it.