-
Notifications
You must be signed in to change notification settings - Fork 13
Replace pkg_resources with importlib for Python 3.12+ support #411
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
Conversation
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
This patch will allow running on Python > 3.11. |
|
All test failures seem to be unrelated. I've filed #412 for one of the failures I've spotted. |
ekohl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I don't consider the docs config to be blocking and I need this on my own dev setup so merging this.
| package = pkg_resources.require("obal")[0] | ||
| version = package.version | ||
| release = package.version | ||
| from importlib import metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: added in Python 3.8 (https://docs.python.org/3/library/importlib.metadata.html#module-importlib.metadata)
| release = package.version | ||
| from importlib import metadata | ||
| version = release = metadata.version('obal') | ||
| except: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we scope it explicitly to metadata.PackageNotFoundError now?
I'd be tempted to use
from importlib import metadata
try:
version = release = metadata.version('obal')
except metadata.PackageNotFoundError:
version = release = ''There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check this one out
|
|
||
| import os | ||
| import obsah | ||
| from importlib import resources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: added in Python 3.7 (https://docs.python.org/3/library/importlib.resources.html#module-importlib.resources)
| # https://github.com/pytest-dev/pytest-xdist/issues/414 | ||
| distribution = pkg_resources.get_distribution('obal') | ||
| path = os.path.join(distribution.location, path) | ||
| path = str(resources.files(__name__) / 'data') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: added in Python 3.9 (https://docs.python.org/3/library/importlib.resources.html#importlib.resources.files). This call looks correct for both 3.9 and 3.12+ (where it was changed).
ekohl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind rebasing this and also update the README that we require Python 3.9+?
ekohl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging despite failing CI. That's introduced by https://docs.ansible.com/projects/ansible-core/devel/porting_guides/porting_guide_core_2.19.html#example-implicit-boolean-conversion and tracked in #412.
Summary
• Replace deprecated pkg_resources with modern importlib
• Fix Python 3.12+ compatibility issues
• Update shebang from python2 to python3
Fixes #405
🤖 Generated with Claude Code