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.

Sign up for GitHub

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

Jump to bottom

Leverage importlib.reload for reloading modules #11679

Merged
merged 8 commits into from Sep 13, 2023
Merged

Leverage importlib.reload for reloading modules #11679

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c727fcc
Leverage `importlib.reload` for reloading modules
godlygeek Sep 7, 2023
ca51520
Merge branch 'master' into leverage_importlib_reload
AA-Turner Sep 13, 2023
e909d52
Add escape hatch
AA-Turner Sep 13, 2023
159ac90
Cython
AA-Turner Sep 13, 2023
98c53e0
KeyError
AA-Turner Sep 13, 2023
5320222
Merge branch 'master' into leverage_importlib_reload
AA-Turner Sep 13, 2023
6fb5d9c
CHANGES
AA-Turner Sep 13, 2023
ec974cd
Swap
AA-Turner Sep 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 10 additions & 12 deletions sphinx/ext/autodoc/importer.py
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import contextlib
import importlib
import sys
import traceback
Expand Down Expand Up @@ -84,22 +85,19 @@ def import_object(modname: str, objpath: list[str], objtype: str = '',
while module is None:
try:
orig_modules = frozenset(sys.modules)
import_module(modname, warningiserror=warningiserror)

# Try reloading modules with ``typing.TYPE_CHECKING == True``.
try:
# try importing with ``typing.TYPE_CHECKING == True``
typing.TYPE_CHECKING = True
module = import_module(modname, warningiserror=warningiserror)
except ImportError:
# if that fails (e.g. circular import), retry with
# ``typing.TYPE_CHECKING == False`` after reverting
# changes made to ``sys.modules`` by the failed try
for m in [m for m in sys.modules if m not in orig_modules]:
sys.modules.pop(m)

typing.TYPE_CHECKING = False
module = import_module(modname, warningiserror=warningiserror)
# Ignore failures; we've already successfully loaded these modules.
with contextlib.suppress(Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a narrower exception type than Exception?

Copy link
Contributor Author

@godlygeek godlygeek Sep 8, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so -- at least, not without sacrificing correctness. If I change this to just catch ImportError, then doing:

.. automodule:: pydantic

fails with::

>       _PartialClsOrStaticMethod: TypeAlias = Union[classmethod[Any, Any, Any], staticmethod[Any, Any], partialmethod[Any]]
E       TypeError: type 'classmethod' is not subscriptable

Let me flip the question around on you: given that the module was already imported successfully with typing.TYPE_CHECKING == False, which sorts of exceptions encountered while reloading the module with typing.TYPE_CHECKING == True should cause import_object to fail instead of using the already successfully imported module?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I take it from your testing that e.g. if pydantic is successfully imported, and then fails on the reload, the initial imported module is still usable, coherent, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's my expectation, and it seems to be the case for pydantic, at least up to the point where a basic .. automodule:: pydantic succeeds. I'm not sure how to test it more thoroughly, though. I do suspect that this is still going to be fragile in ways that we can't predict, the hope is just that it works for more cases than both what's in #11511 and what's in #11645

new_modules = frozenset(sys.modules) - orig_modules
for m in new_modules:
importlib.reload(sys.modules[m])
finally:
# ensure ``typing.TYPE_CHECKING == False``
typing.TYPE_CHECKING = False
module = sys.modules[modname]
logger.debug('[autodoc] import %s => %r', modname, module)
except ImportError as exc:
logger.debug('[autodoc] import %s => failed', modname)
Expand Down