Sphinx 7.2 autodoc fails to import some modules that use typing.TYPE_CHECKING

[godlygeek]

Describe the bug

Sphinx 7.2's autodoc sets typing.TYPE_CHECKING = True before importing a module. If that fails, it sets typing.TYPE_CHECKING = False and retries. It's possible for the first failing import to leave things in a state where the second import won't succeed.

How to Reproduce

With a virtualenv with Sphinx installed activated, run:

mkdir -p sphinx_type_checking_issue
cd sphinx_type_checking_issue

mkdir -p package
printf '%s\n' 'from package.c import SomeClass' >package/__init__.py
printf '%s\n' 'X = 42' >package/a.py
printf '%s\n' 'import typing' 'if typing.TYPE_CHECKING: from package import SomeClass' >package/b.py
printf '%s\n' 'import package.a' 'import package.b' 'class SomeClass: X = package.a.X' >package/c.py
printf '%s\n' '.. automodule:: package' >index.rst
python -m sphinx -v -aE -C -D 'extensions=sphinx.ext.autodoc' . output

You'll see that the last command fails if Sphinx >7.2 is installed, and succeeds if Sphinx 7.1 is installed.

Environment Information

Platform:              linux; (Linux-5.15.90.1-microsoft-standard-WSL2-x86_64-with-glibc2.17)
Python version:        3.11.4 (main, Jun  8 2023, 11:52:43) [GCC 10.2.1 20210130 (Red Hat 10.2.1-11)])
Python implementation: CPython
Sphinx version:        7.2.2
Docutils version:      0.20.1
Jinja2 version:        3.1.2
Pygments version:      2.16.1

Sphinx extensions

`sphinx.ext.autodoc`

Additional context

Obviously the minimal reproducer above is pretty contrived, but it's representative of a real error that we saw inside an actual code base.

For the reproducer above, the issue boils down to this: after the first import with typing.TYPE_CHECKING = True, sys.modules has a "package.a" key but not a "package" key. When we do the second import with typing.TYPE_CHECKING = False, the import package.a statement returns the module associated with the "package.a" key in sys.modules, but doesn't assign sys.modules["package"].a = sys.modules["package.a"] like an import ordinarily would, because it believes the module has already been successfully imported. Because of that, the later attribute access of sys.modules["package"].a fails.

I think the sanest thing to do might be to save a shallow copy of sys.modules before trying the import with typing.TYPE_CHECKING = True and, if that import fails, restore the old sys.modules before retrying with typing.TYPE_CHECKING = False. I don't think side effects from the failing TYPE_CHECKING = True import should be visible to the second import attempt.

[AA-Turner]

Hi Matt, thanks for the detailed report! Would you consider opening a PR with your suggested fix please? It seems reasonable to me.

A

[godlygeek]

This is probably crazy, but since I've had the idea I might as well suggest it: another option might be importing the module twice, first with typing.TYPE_CHECKING set to False as it normally would be, and then setting it to True and doing an importlib.reload(). That ought to solve nearly every case of circular imports, I think, because all modules involved in the cycle will have been loaded by the first import, and should be available when it's re-imported. It would mean that you get the benefits of typing.TYPE_CHECKING = True even when there are circular imports. It comes with all the normal downsides of reloading a module, like that if you do from foo import bar before the reload, bar is foo.bar won't be true after the reload - but that probably doesn't actually matter at all to Sphinx, since it isn't trying to run the code, and it would only compare those things by __name__ or __qualname__ and not by identity...

[AA-Turner]

I haven't looked into how importlib.reload() works, but no harm trying it. I can't remember if we have a test case for circular imports, though.

A

[picnixz]

IIRC, one issue with the reload is that it re-executes the module. In particular if you patched the module using an extension I am not entirely sure that the module will be patched again.

[AA-Turner]

Closed in #11645