GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-133656: Remove deprecated `zipimport.zipimporter.load_module` by Wulian233 · Pull Request #133662 · python/cpython

Conversation

@Wulian233

@Wulian233 Wulian233 commented May 8, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Ready review!

  • Issue: Remove deprecated zipimport.zipimporter.load_module #133656

📚 Documentation preview 📚: https://cpython-previews--133662.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Wulian233
Remove deprecated zipimport.zipimporter.load_module
7a33c7a
@Wulian233
Wulian233 requested review from brettcannon, ericsnowcurrently, ncoghlan and warsaw as code owners May 8, 2025 12:54
@bedevere-app bedevere-app Bot mentioned this pull request May 8, 2025
Remove deprecated zipimport.zipimporter.load_module #133656
Closed
@bedevere-app bedevere-app Bot added the awaiting review label May 8, 2025
@sharktide

sharktide commented May 8, 2025

Copy link
Copy Markdown
Contributor

@Wulian233

considering how pyhton imports work, You might have to modify the files that run on import or any references to load_module. This will be a really daunting task for you :( sorry!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland
StanFromIreland reviewed May 8, 2025
View reviewed changes
Comment thread Doc/whatsnew/3.15.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland
StanFromIreland reviewed May 8, 2025
View reviewed changes
Comment thread Doc/deprecations/pending-removal-in-3.15.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Wulian233
Fix test failure
02919a6
@sharktide

sharktide commented May 10, 2025

Copy link
Copy Markdown
Contributor

@Wulian233 Will review tmrw
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sharktide
sharktide reviewed May 10, 2025
View reviewed changes

@sharktide sharktide left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

This PR seems sound, however I'll approve it after stanfromireland and hugovk resolve their conversation, because I am not sure either if it should or shouldn't be in the devguide either :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Wulian233 added 6 commits May 11, 2025 20:18
@Wulian233
Merge branch 'main' into zipimporter
f95d624
@Wulian233
Update pending-removal-in-3.15.rst
ead7345
@Wulian233
Update pending-removal-in-3.15.rst
963343d
@Wulian233
Merge branch 'main' into zipimporter
91afdcb
@Wulian233
Update 3.15.rst
0edaa29
@Wulian233
Resolved conflict
5616e27
ncoghlan
ncoghlan requested changes Aug 2, 2025
View reviewed changes

@ncoghlan ncoghlan left a comment

Copy link
Copy Markdown
Contributor

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.

There is some additional sys.path manipulation added to the test case that doesn't look like it should be necessary.

Otherwise this looks good to me.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_zipimport.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_zipimport.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting review labels Aug 2, 2025
@bedevere-app

bedevere-app Bot commented Aug 2, 2025

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Wulian233

Wulian233 commented Aug 3, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There is some additional sys.path manipulation added to the test case that doesn't look like it should be necessary.

Otherwise this looks good to me.

That's necessary. If these two lines are removed, the test will fail (p1)
2025-08-03 132623

with a ModuleNotFoundError because the modules cannot be found. If they are added back, the test passes. We need to make sure Python can find them, which maybe is the negative effect of removing load_module? I'm not an expert for this, and I don't know the underlying reasons for them, sorry

And sys.path.insert is also used in many other places in this file, for example:

https://github.com/Wulian233/cpython/blob/5616e27403799b481f44f96c3a21336fd03941c9/Lib/test/test_zipimport.py#L165
屏幕截图 2025-08-03 134313

https://github.com/Wulian233/cpython/blob/5616e27403799b481f44f96c3a21336fd03941c9/Lib/test/test_zipimport.py#L373

https://github.com/Wulian233/cpython/blob/5616e27403799b481f44f96c3a21336fd03941c9/Lib/test/test_zipimport.py#L420-L422
屏幕截图 2025-08-03 134236
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Wulian233
Wulian233 requested a review from ncoghlan August 11, 2025 02:03
@ncoghlan

ncoghlan commented Aug 11, 2025

Copy link
Copy Markdown
Contributor

@Wulian233 You're right, the importlib.import_module lines were previously only passing because the prior zi.load_module call had left sys.modules["TESTPACK"] populated.

Given that, it makes sense to keep the path manipulation additions, but move them as far down in their respective test cases as possible to still let the everything pass (which should be just before the importlib.import_module line in each case, since the direct instantiation shouldn't care if the zip archive is on sys.path or not, while importlib.import_module does care).
gpshead reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Wulian233
move to before the first full import request
6207b71
@Wulian233
Wulian233 requested a review from AA-Turner as a code owner August 11, 2025 13:16
@Wulian233

Wulian233 commented Aug 11, 2025

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting changes label Aug 11, 2025
@bedevere-app bedevere-app Bot added the awaiting change review label Aug 11, 2025
@bedevere-app

bedevere-app Bot commented Aug 11, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

@ncoghlan: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead
Merge branch 'main' into zipimporter
51373ed
gpshead
gpshead approved these changes Aug 30, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting change review labels Aug 30, 2025
@gpshead
gpshead enabled auto-merge (squash) August 30, 2025 01:08
@gpshead
gpshead merged commit 5c6937a into python:main Aug 30, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 30, 2025
@bedevere-bot

bedevere-bot commented Aug 30, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 Raspbian 3.x (tier-3) has failed when building commit 5c6937a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1678/builds/742) and take a look at the build logs.
  4. Check if the failure is related to this commit (5c6937a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1678/builds/742

Failed tests:

  • test.test_multiprocessing_fork.test_processes

Failed subtests:

  • test_interrupt - test.test_multiprocessing_fork.test_processes.WithProcessesTestProcess.test_interrupt

Summary of the results of the build (if available):

==

Click to see traceback logs 
Traceback (most recent call last):
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/contextlib.py", line 85, in inner
    return func(*args, **kwds)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 596, in test_interrupt
    exitcode = self._kill_process(multiprocessing.Process.interrupt)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/contextlib.py", line 85, in inner
    return func(*args, **kwds)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 577, in _kill_process
    self.assertEqual(join(), None)
                     ~~~~^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 250, in __call__
    return self.func(*args, **kwds)
           ~~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/multiprocessing/process.py", line 156, in join
    res = self._popen.wait(timeout)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/multiprocessing/popen_fork.py", line 44, in wait
    return self.poll(os.WNOHANG if timeout == 0.0 else 0)
           ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/multiprocessing/popen_fork.py", line 28, in poll
    pid, sts = os.waitpid(self.pid, flag)
               ~~~~~~~~~~^^^^^^^^^^^^^^^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 573, in handler
    raise RuntimeError('join took too long: %s' % p)
RuntimeError: join took too long: <Process name='Process-1' pid=226230 parent=226228 started daemon>


Traceback (most recent call last):
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/contextlib.py", line 85, in inner
    return func(*args, **kwds)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 596, in test_interrupt
    exitcode = self._kill_process(multiprocessing.Process.interrupt)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/contextlib.py", line 85, in inner
    return func(*args, **kwds)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 577, in _kill_process
    self.assertEqual(join(), None)
                     ~~~~^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 250, in __call__
    return self.func(*args, **kwds)
           ~~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/multiprocessing/process.py", line 156, in join
    res = self._popen.wait(timeout)
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/multiprocessing/popen_fork.py", line 44, in wait
    return self.poll(os.WNOHANG if timeout == 0.0 else 0)
           ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/multiprocessing/popen_fork.py", line 28, in poll
    pid, sts = os.waitpid(self.pid, flag)
               ~~~~~~~~~~^^^^^^^^^^^^^^^^
  File "/home/stan/buildarea/3.x.stan-raspbian.nondebug/build/Lib/test/_test_multiprocessing.py", line 573, in handler
    raise RuntimeError('join took too long: %s' % p)
RuntimeError: join took too long: <Process name='Process-151' pid=210285 parent=209827 started daemon>
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
@Wulian233 @lkollar
pythongh-133656: Remove deprecated zipimport.zipimporter.load_module (
1ab4544 
pythonGH-133662)

Remove deprecated `zipimport.zipimporter.load_module`.
@Wulian233
Wulian233 deleted the zipimporter branch September 27, 2025 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk left review comments
@StanFromIreland StanFromIreland StanFromIreland left review comments
@gpshead gpshead gpshead approved these changes
@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
+1 more reviewer
@sharktide sharktide sharktide left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@Wulian233 @sharktide @ncoghlan @bedevere-bot @gpshead @hugovk @StanFromIreland
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.