-
Couldn't load subscription status.
- Fork 931
update module importer #2636
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
base: master
Are you sure you want to change the base?
update module importer #2636
Conversation
e7b37b5 to
3fc7718
Compare
| raise ImportError(f"No module named '{fullname}'") | ||
|
|
||
| # Replace the standard module with the wrapped module in sys.modules | ||
| sys.modules[fullname] = wrapped_module |
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.
Looking at the code of how this is used in the import system, I believe we shouldn't be adding stuff to sys.modules. Maybe we can try to do more in create_module and return wrapped_module there and then not do anything in exec_module. I think exec_module is also for reloading which we do not support much right now.
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.
That makes sense. Originally I was trying not to change the existing logic too much, but I've updated it to have create_module handle everything and it seems much better. Thanks for the help!
f70b5a2 to
ed9060d
Compare
|
Netflix internal testing[1388] @ ed9060d |
ed9060d to
f8ee813
Compare
f8ee813 to
05cc612
Compare
Update the ModuleImporter class to use
find_specinstead offind_moduleandcreate_moduleandexec_moduleinstead ofload_module. In Python 3.12, the fallback that looked for afind_module()method if a meta_path entry didn't have afind_spec()method was removed, so the escape hatch didn't work correctly in this version when running with--environment=conda.