Fix viewcode extension importing modules more than once #13380
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
djhoese
commented
Feb 21, 2025
Comment on lines
+71
to
+75
| except BaseException as exc: | ||
| # Importing modules may cause any side effects, including | ||
| # SystemExit, so we need to catch all errors. | ||
| msg = f"viewcode failed to import '{mod_root}'." | ||
| raise ImportError(msg) from exc |
There was a problem hiding this comment.
@AA-Turner There is an outer except Exception, does that count?
There was a problem hiding this comment.
This has been wrong for a while, simple counterexample:
import sys
try:
sys.exit()
except Exception:
print('caught')|
Thanks @djhoese! A |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
This PR fixes the viewcode extension improperly importing modules to compute the full module name. This results in modules being imported more than once and increase memory usage and execution time. The primary issue was modules being imported/executed but not added to
sys.modulesto be reused.This PR reverts some of the changes in #13195 to use
importlib.import_moduleagain to handle the complexity of importing modules and "caching" them insys.modulesto be reused. It does not break the feature added to #13195 of importing a module "alias" import. That is, accessing an imported module as another module's attribute (see #13195 and #13370 for details).References