-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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.
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
bpo-34871: inspect: Don't pollute sys.modules #9696
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the idea to copy sys.modules
. It's an expensive operation, and this code is executed to evaluate every parameter with a non-trivial default for a function with __text_signature__
.
Looking at the documentation of Argument Clinic, I see that this code is supposed to handle simple defaults like mod.attr
.
So why don't we rewrite wrap_value
to do just that, e.g.:
def wrap_value(s):
try:
value = eval(s, module_dict)
except NameError:
match = re.match(r'\s*(?P<mod>\w+)\.(?P<attr>\w+)\s*', s)
if not match:
raise RuntimeError
mod = sys.modules.get(match.group('mod'))
if mod is None:
raise RuntimeError
try:
value = getattr(mod, match.group('attr'))
except AttributeError:
raise RuntimeError
This code is equivalent to the previous code and doesn't use eval()
at all for evaluating non-trivial defaults.
When you're done making the requested changes, leave the comment: |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sys.modules is a plain dict. Copying a dict will be faster than regular expression.
Depends on how many modules you have in your application. But your solution to copy it once works for me.
@methane: Status check is done, and it's a success ✅ . |
Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7. |
https://bugs.python.org/issue34871 (cherry picked from commit 6f85b82) Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
GH-9701 is a backport of this pull request to the 3.7 branch. |
https://bugs.python.org/issue34871 (cherry picked from commit 6f85b82) Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
GH-9702 is a backport of this pull request to the 3.6 branch. |
https://bugs.python.org/issue34871 (cherry picked from commit 6f85b82) Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
https://bugs.python.org/issue34871 (cherry picked from commit 6f85b82) Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
https://bugs.python.org/issue34871