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

core: replace pkg_resources with importlib.metadata #2261

Merged
merged 7 commits into from
Mar 1, 2022
Merged

Conversation

dgw
Copy link
Member

@dgw dgw commented Feb 26, 2022

Description

Tin. Performance of pkg_resources can be quite slow (just search all of GitHub issues for "pkg_resources slow"); its use is discouraged by the setuptools project itself, in favor of the newer modules such as importlib.metadata.

Needs a backport package until we no longer support Python 3.9. stdlib gained importlib.metadata in Python 3.8, but the ability to "select" in entry_points() won't be added until Python 3.10. I don't mind this, because it takes an implied dependency (setuptools was required to install Sopel but not listed in requirements.txt) and makes it explicit until we can use the stdlib version. My requirement specifier gives the minimum version of the importlib_metadata backport that supports selectors.

I didn't see a direct equivalent to pkg_resources.parse_version() in importlib.metadata; looking at a couple other projects that have already made the move away from pkg_resources told me that packaging.version.parse() is the way to go.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
    • I will eagerly await @Exirel telling me what I did wrong when updating the tests. 😁
  • I have tested the functionality of the things this change touches
    • In addition to the test suite, I also installed and used sopel-dns on my local (-e .) SopelTest instance with no issues.

Notes

We devs/hackers have been installing importlib_metadata for a while without thinking about it, because Sphinx explicitly requires it for install on py<3.10.

dgw added 4 commits February 26, 2022 12:53
Needs a backport package until we no longer support Python 3.7; stdlib
gained `importlib.metadata` in Python 3.8.

I should note that installing with `-r dev-requirements.txt` will always
(currently) install the `importlib_metadata` backport because of Sphinx.
Unfortunately this one is *not* yet part of the stdlib in any Python
release, but there's no point in removing only *most* uses of the older
`pkg_resources` library. Gotta do them *all*.
Adapted the `get_meta_description()` for `EntryPointPlugin` to return
the expected format. Could alternatively change the expectation of
`test_get_label_entrypoint()`.
@dgw dgw added Housekeeping Code cleanup, removal of deprecated stuff, etc. Core/Plugin Handling labels Feb 26, 2022
@dgw dgw added this to the 8.0.0 milestone Feb 26, 2022
@dgw dgw requested a review from a team February 26, 2022 23:39
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

Impressive work, having to deal with setuptools, metadata, and backport packages for Python version up to 3.9 is no small feat. Well done!

I don't approve yet because of one comment, however, it's only for documentation.

sopel/plugins/handlers.py Show resolved Hide resolved
sopel/plugins/__init__.py Show resolved Hide resolved
As the Python packaging docs say, entry points are now a PyPA-defined
interoperability specification not confined to packages built using
`setuptools`, and we should update our docs to reflect that.

See https://packaging.python.org/en/latest/specifications/entry-points/
@dgw dgw requested a review from Exirel February 28, 2022 00:53
@dgw
Copy link
Member Author

dgw commented Feb 28, 2022

@Exirel I went a bit ham on updating the docs instead of just tweaking the one setuptools reference you pointed out. 😅

Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

I'm clearly not going to nitpick at grammar, this looks pretty good to me.

@dgw dgw merged commit aca1055 into master Mar 1, 2022
@dgw dgw deleted the remove-pkg_resources branch March 1, 2022 01:51
@dgw
Copy link
Member Author

dgw commented Mar 13, 2022

Followed up with #2268, since the importlib_metadata reality isn't quite as elegant as the theory this PR was based on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core/Plugin Handling Documentation Housekeeping Code cleanup, removal of deprecated stuff, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants