Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-40173: Remove redundant import module of import_fresh_module()#19390

Closed
shihai1991 wants to merge 1 commit into
python:mainpython/cpython:mainfrom
shihai1991:bpo_40173shihai1991/cpython:bpo_40173Copy head branch name to clipboard
Closed

bpo-40173: Remove redundant import module of import_fresh_module()#19390
shihai1991 wants to merge 1 commit into
python:mainpython/cpython:mainfrom
shihai1991:bpo_40173shihai1991/cpython:bpo_40173Copy head branch name to clipboard

Conversation

@shihai1991

@shihai1991 shihai1991 commented Apr 6, 2020

Copy link
Copy Markdown
Member

@shihai1991 shihai1991 changed the title WIP: Remove redundant import module of import_fresh_module() bpo-40173: Remove redundant import module of import_fresh_module() Apr 6, 2020

@iritkatriel iritkatriel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR needs a unit test.

@shihai1991

Copy link
Copy Markdown
Member Author

This PR needs a unit test.

Thanks for your comment. I have forget this PR's detail. test.support need many test cases.
After we make sure this PR can fix the issue, I can add some test cases.

@iritkatriel

Copy link
Copy Markdown
Member

Up to you, but I think it will be easier for reviewers to see what issue the PR is trying to fix if it included tests.

Comment on lines +245 to +246
sys.modules.clear()
sys.modules.update(orig_modules)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I considered this option, but some modules can not support re-importing. So it is safer to restore only explicitly mentioned modules and their submodules.
See #28654.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, sorry, Serhiy. I missed your PR :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.