gh-97850: Deprecate `find_loader` and `get_loader` in `pkgutil` by sobolevn · Pull Request #98520 · python/cpython · GitHub

Issue: Meta issue for cleaning up import system cruft #97850

Some comments and suggestions on the implementation of this deprecation, based on how we've handled others recently.

Pro tip (since not everyone seems to know this): if you go to the Files changed tab of this PR, you can apply some or all of the suggestions in one click by clicking Add to batch on each suggestion you want, and then click Commit either at the top or on any added suggestion with a descriptive message.

…6coo.rst Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>

Thank you for the reviews: @merwok and @CAM-Gerlach
It is much better now. I was not aware of ._deprecated helper.

I've also checked that get_loader does not get 2 warnings (because it is using find_loader inside):

>>> import pkgutil
>>> pkgutil.get_loader('os')
<stdin>:1: DeprecationWarning: 'pkgutil.get_loader' is deprecated and slated for removal in Python 3.14; use importlib.util.find_spec instead
<_frozen_importlib_external.SourceFileLoader object at 0x1012d55f0>
>>>

test_pkgutil tests are also clean, no unwanted warnings. I will look through all logs again when it is completed.

I've also merged one of the @CAM-Gerlach's suggestion, so you get a credit for your awesome proposals! 🤝

Overall LGTM, but I think there might be some unnecessary checks in the tests for various module "kinds".

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.

I've also merged one of the @CAM-Gerlach's suggestion, so you get a credit for your awesome proposals! handshake

Just FYI, you can apply some or all of them at once by going to the Files changed tab, clicking Add to batch on each suggestion and then clicking `Commit (with an appropriate message).

…m/sobolevn/cpython into deprecate-pkgutil-get-find-loader

@brettcannon I forgot to send the magic phrase!

I have made the requested changes; please review again

Thanks for making the requested changes!

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

Unfortunately I just realized that the documentation for pkgutil needs updating to mention the deprecations.

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.

Co-authored-by: Brett Cannon <brett@python.org>

…6coo.rst Co-authored-by: Brett Cannon <brett@python.org>

I have made the requested changes; please review again 🙂

Thanks for making the requested changes!

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

Oups, just found out that my push with pkgutil.rst change did not make it into this branch :(

Redid it now, it worked!

@sobolevn the changes look good, but now there are unfortunately merge conflicts.

Done :)

Thanks!

Thank you for this additional cleanup!

pythongh-97850 : Deprecate find_module and get_module in pkgutil

b7de6df

bedevere-bot added the awaiting review label Oct 21, 2022

sobolevn mentioned this pull request Oct 21, 2022

Meta issue for cleaning up import system cruft #97850

Open

25 of 31 tasks

JelleZijlstra requested a review from brettcannon October 26, 2022 03:52

merwok reviewed Nov 8, 2022

View reviewed changes

Comment thread Lib/pkgutil.py Outdated

CAM-Gerlach reviewed Nov 8, 2022

View reviewed changes

Comment thread Doc/whatsnew/3.12.rst Outdated

sobolevn and others added 2 commits November 9, 2022 16:22

Address review

b1e6d4d

Update Misc/NEWS.d/next/Library/2022-10-21-16-23-31.gh-issue-97850.N4…

d55a9b7

…6coo.rst Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>

brettcannon requested changes Nov 25, 2022

View reviewed changes

Comment thread Lib/test/test_pkgutil.py Outdated

bedevere-bot added awaiting changes and removed awaiting review labels Nov 25, 2022

sobolevn added 2 commits November 26, 2022 13:13

Use just one module

e282911

Merge branch 'deprecate-pkgutil-get-find-loader' of https://github.co…

4f9a697

…m/sobolevn/cpython into deprecate-pkgutil-get-find-loader

kdeldycke mentioned this pull request Jan 2, 2023

Python 3.12-dev & Windows: zipimporter has no attribute find_module pypa/pip#11688

Closed

1 task done

bedevere-bot added awaiting change review and removed awaiting changes labels Jan 7, 2023

bedevere-bot requested a review from brettcannon January 7, 2023 08:41

Merge branch 'main' into deprecate-pkgutil-get-find-loader

4222f23

brettcannon requested changes Jan 21, 2023

View reviewed changes

Comment thread Doc/whatsnew/3.12.rst Outdated

Comment thread Misc/NEWS.d/next/Library/2022-10-21-16-23-31.gh-issue-97850.N46coo.rst Outdated

bedevere-bot added awaiting changes and removed awaiting change review labels Jan 21, 2023

Apply suggestions from code review

817f2da

Co-authored-by: Brett Cannon <brett@python.org>

sobolevn changed the title gh-97850: Deprecate find_module and get_module in pkgutil gh-97850: Deprecate find_loader and get_loader in pkgutil Feb 5, 2023

sobolevn and others added 2 commits February 5, 2023 14:59

Update Misc/NEWS.d/next/Library/2022-10-21-16-23-31.gh-issue-97850.N4…

8b116d9

…6coo.rst Co-authored-by: Brett Cannon <brett@python.org>

Merge branch 'main' into deprecate-pkgutil-get-find-loader

e94a6f3

bedevere-bot added awaiting change review and removed awaiting changes labels Apr 11, 2023

bedevere-bot requested a review from brettcannon April 11, 2023 07:30

Mention deprecation in pkgutil docs

e073fac

hugovk reviewed Apr 19, 2023

View reviewed changes

Comment thread Doc/library/pkgutil.rst Outdated

Comment thread Doc/library/pkgutil.rst Outdated

Comment thread Doc/whatsnew/3.12.rst Outdated

sobolevn and others added 2 commits April 19, 2023 15:57

Address review

1e26981

Merge branch 'main' into deprecate-pkgutil-get-find-loader

24badea

brettcannon reviewed May 3, 2023

View reviewed changes

Comment thread Lib/pkgutil.py Outdated

Comment thread Lib/pkgutil.py Outdated

Apply suggestions from code review

619c4dc

brettcannon approved these changes May 3, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels May 3, 2023

Merge branch 'main' into deprecate-pkgutil-get-find-loader

b65576a

brettcannon merged commit d6e83fb into python:main May 3, 2023

bedevere-bot removed the awaiting merge label May 3, 2023

Wulian233 mentioned this pull request Jun 16, 2024

Remove pkgutil.get_loader and pkgutil.findloader #120581

Closed

gh-97850: Deprecate `find_loader` and `get_loader` in `pkgutil` by sobolevn · Pull Request #98520 · python/cpython · GitHub

Repository navigation

gh-97850: Deprecate find_loader and get_loader in pkgutil - #98520

gh-97850: Deprecate find_loader and get_loader in pkgutil#98520brettcannon merged 14 commits intopython:mainpython/cpython:mainfrom sobolevn:deprecate-pkgutil-get-find-loadersobolevn/cpython:deprecate-pkgutil-get-find-loaderCopy head branch name to clipboard

Conversation

sobolevn commented Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

CAM-Gerlach left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sobolevn commented Nov 9, 2022

Uh oh!

brettcannon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bedevere-bot commented Nov 25, 2022

Uh oh!

CAM-Gerlach commented Nov 26, 2022

Uh oh!

sobolevn commented Jan 7, 2023

Uh oh!

bedevere-bot commented Jan 7, 2023

Uh oh!

brettcannon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Jan 21, 2023

Uh oh!

sobolevn commented Apr 11, 2023

Uh oh!

bedevere-bot commented Apr 11, 2023

Uh oh!

sobolevn commented Apr 19, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brettcannon commented May 3, 2023

Uh oh!

sobolevn commented May 3, 2023

Uh oh!

brettcannon commented May 3, 2023

Uh oh!

warsaw commented May 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gh-97850: Deprecate find_loader and get_loader in pkgutil#98520
brettcannon merged 14 commits into
python:mainfrom
sobolevn:deprecate-pkgutil-get-find-loader

sobolevn commented Oct 21, 2022 •
edited
Loading

CAM-Gerlach left a comment •
edited
Loading