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

replace pkg_resources with importlib.metadata from stdlib #3333

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

megies
Copy link
Member

@megies megies commented Jul 25, 2023

What does this PR do?

Replaces pkg_resources which was used internally to handle entry points / plugin structure with importlib.metadata API. The EntryPoint objects from importlib (and other objects like Distribution all have very minor differences to the old pkg_resources objects but most of these routines are private and not intended for public use. Hopefully tests catch all use cases.

Why was it initiated? Any relevant Issues?

pkg_resources recently started showing this message:

DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
  • While the PR is still work-in-progress, the no_ci label can be added to skip CI builds
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just add the build_docs tag to this PR.
    Docs will be served at docs.obspy.org/pr/{branch_name} (do not use master branch).
    Please post a link to the relevant piece of documentation.
  • If all tests including network modules (e.g. clients.fdsn) should be tested for the PR,
    just add the test_network tag to this PR.
  • All tests still pass.
  • Any new features or fixed regressions are covered via new tests.
  • Any new or changed features are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .
  • If the changes affect any plotting functions you have checked that the plots
    from all the CI builds look correct. Add the "upload_plots" tag so that plotting
    outputs are attached as artifacts.
  • New modules, add the module to CODEOWNERS with your github handle
  • Add the yellow ready for review label when you are ready for the PR to be reviewed.

@megies megies added installation issues with installing obspy cleanup code refactoring etc. labels Jul 25, 2023
@megies megies added this to the 1.5.0 milestone Jul 25, 2023
@megies megies force-pushed the replace_pkg_resources_with_importlib branch from df1c0ab to 5e36da7 Compare July 25, 2023 12:23
@ThomasLecocq
Copy link
Contributor

i wonder if this is going to make a difference in load-time of obspy :-)

@megies megies force-pushed the replace_pkg_resources_with_importlib branch from 5e36da7 to fa1978c Compare July 25, 2023 12:37
@megies
Copy link
Member Author

megies commented Jul 25, 2023

i wonder if this is going to make a difference in load-time of obspy :-)

No idea, just wanted to act on that DeprecationWarning from pkg_resources 😉

Rebased after merging current maintenance branch into master

@megies
Copy link
Member Author

megies commented Jul 25, 2023

Looks like importlib.metadata in Python 3.8 and 3.9 still has some reduced API for entry points. If we keep following numpy we'd drop Python 3.8 anyway for 1.5.0, but that would still leave Python 3.9 until next April or so.
So if we wanna go through with this now we'd have to put some compatibility layer in there that emulates the missing filtering logic. It might be some minor workarounds only needed, but not sure if it's worth pushing this now, maybe just leave this open and bump it to 1.6.0 or some other release coming next year.

  File "/usr/share/miniconda3/envs/test/lib/python3.9/site-packages/obspy/core/util/__init__.py", line 22, in <module>
    from obspy.core.util.base import (ALL_MODULES, DEFAULT_MODULES,
  File "/usr/share/miniconda3/envs/test/lib/python3.9/site-packages/obspy/core/util/base.py", line 280, in <module>
    'trigger': _get_entry_points('obspy.plugin.trigger'),
  File "/usr/share/miniconda3/envs/test/lib/python3.9/site-packages/obspy/core/util/base.py", line 251, in _get_entry_points
    ep.name: ep for ep in importlib.metadata.entry_points(group=group)}
TypeError: entry_points() got an unexpected keyword argument 'group'
Error: Process completed with exit code 1.

@megies megies force-pushed the replace_pkg_resources_with_importlib branch from fa1978c to 5450d4e Compare October 27, 2023 10:18
@megies megies force-pushed the replace_pkg_resources_with_importlib branch from cdd4f64 to c2657b7 Compare October 27, 2023 15:41
@megies
Copy link
Member Author

megies commented Oct 30, 2023

This is starting to get ugly with the workarounds needed for Py 3.8 and 3.9. Maybe best make this switch after dropping Py 3.9 and until then just add an ignore rule for that deprecation warning on Py 3.12

Comment on lines +243 to +247
# can not find proper documentation for how exactly these requirement
# strings are defined, but it looks like a semicolon is used as a
# separator for additional definitions like "extra" in optional
# dependencies and old pkg_resources ignored those, so skip them with
# importlib too
Copy link
Member

Choose a reason for hiding this comment

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

I think it follows PEP508; you could parse it with packaging.

format_ep.dist.key, 'obspy.plugin.event.%s' % (format_ep.name),
'writeFormat')
format_ep.dist.name,
'obspy.plugin.event.%s' % (format_ep.name), 'writeFormat')
Copy link
Member

Choose a reason for hiding this comment

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

Are we able to change all of these to f-strings? There are quite a few of these, and they're all a bit different, such as this one having superfluous parentheses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup code refactoring etc. installation issues with installing obspy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants