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

migrate importlib to resolve warning about pkg_resources #3061

Merged
merged 7 commits into from
Aug 30, 2024

Conversation

raspofabs
Copy link
Contributor

@raspofabs raspofabs commented Aug 18, 2024

I came across this warning when running pytest. I wondered if this fix would be appropriate.
At least on my machine the warning goes away and satisfies the deprecation warning from https://setuptools.pypa.io/en/latest/pkg_resources.html

I think I'm only using this feature for Fonts, so don't know how to check if I have broken anything with this change.

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

Hi. First of all, thanks for contributing! 🎉

We just need to handle the python 3.8 separately in an if condition, using importlib.resources.open_binary and importlib.resources.is_resource for that case.

src_py/pkgdata.py Outdated Show resolved Hide resolved
src_py/pkgdata.py Outdated Show resolved Hide resolved
@ankith26
Copy link
Member

Also, we could potentially be looking into dropping python 3.8 support soon now that it's approaching EOL. That should make the implementation of this PR simpler

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

Yes, this is the approach I'm suggesting.

Except the CI seems to be failing, IG we have to do _package_or_requirement = _package_or_requirement.split(".")[0] in all the functions because we need it to be relative to the main pygame package and not pygame.pkgdata submodule.

src_py/pkgdata.py Outdated Show resolved Hide resolved
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

Almost there... Just gotta add # pylint: disable=deprecated-method; comment in front of the deprecated functions to make pylint happy

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM, would be good to clear this warning up somewhat from the output (even if we are not really using this file much anymore). I'm familiar with the replacement package and use it for the same purposes in pygame GUI.

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing! 🥳

@ankith26 ankith26 added this to the 2.5.2 milestone Aug 30, 2024
@ankith26 ankith26 merged commit 06177e7 into pygame-community:main Aug 30, 2024
24 checks passed
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.

3 participants