Skip to content
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

gh-125746: Delay deprecated zipimport.zipimporter.load_module removal time to 3.15 #125748

Merged
merged 8 commits into from
Feb 11, 2025

Conversation

Wulian233
Copy link
Contributor

@Wulian233 Wulian233 commented Oct 20, 2024

#125746 (comment)

I would like to hold off on this change for one more version. A quick GitHub search shows that 1.8k projects are still relying on load_module (and my regular expression isn't perfect, so I'm certain that we're missing some).

The new zipimport methods (I'm using "new" loosely here, I'm talking about the 3.10 methods) were only added to typeshed in late June, so I would assume that many were intentionally using load_module over exec_module, because their IDE was telling them that it doesn't exist (to confirm that, adjusting my GitHub search shows that only 250 projects are using exec_module). Similarly, load_module isn't marked as @deprecated on typeshed, so when we did add it, it's unlikely that people realized they needed to migrate.


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

@rruuaanng
Copy link
Contributor

I also encountered this problem, but I don't know why it happened. I opened an issue.

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

I would like to postpone this until 3.15. Please update the PR to move the load_module deprecation to pending-removal-in-3.15

@Wulian233 Wulian233 changed the title gh-125746: Remove deprecated zipimport.zipimporter.load_module gh-125746: Delay deprecated zipimport.zipimporter.load_module removal time to 3.15 Oct 20, 2024
@Wulian233
Copy link
Contributor Author

You're right, when I did this pr, my vscode (the latest Python extension) didn't prompt exec_module either. I will edit the pending-removal-in-3.15.rst and Lib/zipimport.py to remove the entries for pending-removal-in-future.rst

@Wulian233 Wulian233 reopened this Oct 21, 2024
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM.

@bedevere-app
Copy link

bedevere-app bot commented Oct 21, 2024

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.

@bedevere-app
Copy link

bedevere-app bot commented Oct 21, 2024

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested a review from brettcannon October 21, 2024 22:54
Co-authored-by: Brett Cannon <brett@python.org>
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Can you check if there is a deprecated-removed directive to update in the corresponding docs entry?

@Wulian233
Copy link
Contributor Author

Wulian233 commented Nov 17, 2024

@brettcannon Friendly ping:) maybe we can merge this? It should be fine now, thank you!

@rruuaanng
Copy link
Contributor

@Wulian233 Be patient, everyone has their own job :)

@ZeroIntensity
Copy link
Member

ZeroIntensity commented Nov 17, 2024

Eh, IMO it's fine to remind someone to merge if the PR hasn't had any additional reviewers.

@Wulian233
Copy link
Contributor Author

Wulian233 commented Feb 11, 2025

A few months later: A similar PR for the delayed removal of load_module in importlib #129855 was just merged and backported. Is this PR still needed?

cc @brettcannon

@tomasr8
Copy link
Member

tomasr8 commented Feb 11, 2025

Is this PR still needed?

It is, this one updates zipimport, while the other one was for importlib.

@brettcannon brettcannon enabled auto-merge (squash) February 11, 2025 23:46
@brettcannon brettcannon merged commit 06ac157 into python:main Feb 11, 2025
39 checks passed
@brettcannon
Copy link
Member

Sorry for the delay! Got swamped and this fell off my radar thinking I was waiting on something for the PR.

@Wulian233 Wulian233 deleted the zipimport branch February 12, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants