gh-94379: Remove zipimport find_loader() and find_module() methods#94380
gh-94379: Remove zipimport find_loader() and find_module() methods#94380vstinner merged 1 commit intopython:mainfrom vstinner:zipimport
Conversation
|
cc @brettcannon |
brettcannon
left a comment
There was a problem hiding this comment.
The code removal looks fine, but the doc changes left out the removal of load_module() as well.
|
When you're done making the requested changes, leave the comment: |
|
@brettcannon: I updated the PR to mention also load_module() removal, and fix test_zipimport... But now, I feel that I removed too many lines in the tests! Maybe the tests should be fixed instead? Problem: I don't know how to fix the packdir2 tests: |
Doc/whatsnew/3.12.rst
Outdated
There was a problem hiding this comment.
find_spec() only replaces find_loader() and find_module(), so this sentence is a bit misleading. Same is true for the news entry.
Lib/test/test_zipimport.py
Outdated
There was a problem hiding this comment.
Why the test removals for lines that don't directly use the associated methods?
How is it failing? Maybe only do the |
Good idea! I modified this PR to only remove find_loader() and find_module() methods: keep load_module() for now (it can be removed in a separated PR). |
|
@brettcannon: Would you mind to review the updated PR? Now it only removes find_loader() and find_module() methods, and it only removes the bare minimum lines in test_zipimport.py. Once the PR gets approved, I will solve the Doc/whatsnew/3.12.rst conflict. |
|
@vstinner LGTM! And an FYI for anyone else who reviews, the |
zipimport: Remove find_loader() and find_module() methods, deprecated in Python 3.10: use the find_spec() method instead. See PEP 451 for the rationale.
|
Thanks for the reviews @warsaw and @brettcannon! It's easier to modify zipimport since @serhiy-storchaka rewrote it in pure Python :-) |
zipimport: Remove find_loader() and find_module() methods, deprecated
in Python 3.10. Use the find_spec()` method instead. See PEP 451 for
the rationale.