Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow use of importlib.metadata for finding entrypoints #1102

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

akx
Copy link
Member

@akx akx commented Jul 18, 2024

Closes #1093 (supersedes it, based on it).
Refs #861.

This PR makes Babel attempt to use importlib.metadata instead of pkg_resources where available.

It also adds a test that proves the Jinja2 extractor also works on Python 3.12; this is ran in CI as well.

@akx akx force-pushed the py312-importlib-metadata branch 2 times, most recently from 055e115 to 6017ad0 Compare July 18, 2024 08:21
@akx akx marked this pull request as ready for review July 18, 2024 08:54
@akx
Copy link
Member Author

akx commented Jul 18, 2024

cc @tomasr8 @podgorniy94

@akx akx added this to the Babel 2.16 milestone Jul 18, 2024
@akx akx force-pushed the py312-importlib-metadata branch from 6017ad0 to 6395653 Compare July 18, 2024 09:07
Copy link

codecov bot commented Jul 18, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.21%. Comparing base (42d793c) to head (7a029e0).

Files Patch % Lines
babel/messages/_compat.py 78.94% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1102      +/-   ##
==========================================
- Coverage   91.29%   91.21%   -0.08%     
==========================================
  Files          26       27       +1     
  Lines        4479     4496      +17     
==========================================
+ Hits         4089     4101      +12     
- Misses        390      395       +5     
Flag Coverage Δ
macos-12-3.10 89.99% <75.00%> (-0.06%) ⬇️
macos-12-3.11 89.99% <75.00%> (-0.06%) ⬇️
macos-12-3.12 90.16% <75.00%> (-0.17%) ⬇️
macos-12-3.8 89.91% <65.62%> (-0.12%) ⬇️
macos-12-3.9 89.91% <65.62%> (-0.12%) ⬇️
macos-12-pypy3.10 89.99% <75.00%> (-0.06%) ⬇️
ubuntu-22.04-3.10 90.01% <75.00%> (-0.06%) ⬇️
ubuntu-22.04-3.11 90.01% <75.00%> (-0.06%) ⬇️
ubuntu-22.04-3.12 90.19% <75.00%> (-0.17%) ⬇️
ubuntu-22.04-3.8 89.93% <65.62%> (-0.12%) ⬇️
ubuntu-22.04-3.9 89.93% <65.62%> (-0.12%) ⬇️
ubuntu-22.04-pypy3.10 90.01% <75.00%> (-0.06%) ⬇️
windows-2022-3.10 90.13% <75.00%> (-0.06%) ⬇️
windows-2022-3.11 90.13% <75.00%> (-0.06%) ⬇️
windows-2022-3.12 90.31% <75.00%> (-0.17%) ⬇️
windows-2022-3.8 90.06% <65.62%> (-0.12%) ⬇️
windows-2022-3.9 90.06% <65.62%> (-0.12%) ⬇️
windows-2022-pypy3.10 90.13% <75.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 12 to 34
try:
from importlib.metadata import entry_points
except ImportError:
pass
else:
eps = entry_points()
if isinstance(eps, dict): # Old structure before Python 3.10
group_eps = eps.get(group_name, [])
else: # New structure in Python 3.10+
group_eps = (ep for ep in eps if ep.group == group_name)
for entry_point in group_eps:
yield (entry_point.name, entry_point.load)
return

try:
from pkg_resources import working_set
except ImportError:
pass
else:
for entry_point in working_set.iter_entry_points(group_name):
yield (entry_point.name, partial(entry_point.load, require=True))
Copy link
Contributor

@podgorniy94 podgorniy94 Jul 18, 2024

Choose a reason for hiding this comment

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

As my colleague @j123b567 rightly noted, for versions before 3.10, it is better to use pkg_resources, as the importlib.metadata API is still unstable.
You can use pkg_resources as the first handler, and in the second try block, ensure to verify that the eps object is not a dictionary before processing entry points. Otherwise, simply continue the function execution.
Consider this option as well.

Copy link
Member

Choose a reason for hiding this comment

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

Could you post this as a code suggestion so it's easier to see what you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
try:
from importlib.metadata import entry_points
except ImportError:
pass
else:
eps = entry_points()
if isinstance(eps, dict): # Old structure before Python 3.10
group_eps = eps.get(group_name, [])
else: # New structure in Python 3.10+
group_eps = (ep for ep in eps if ep.group == group_name)
for entry_point in group_eps:
yield (entry_point.name, entry_point.load)
return
try:
from pkg_resources import working_set
except ImportError:
pass
else:
for entry_point in working_set.iter_entry_points(group_name):
yield (entry_point.name, partial(entry_point.load, require=True))
try:
from pkg_resources import working_set
except ImportError:
pass
else:
for entry_point in working_set.iter_entry_points(group_name):
yield (entry_point.name, partial(entry_point.load, require=True))
return
try:
from importlib.metadata import entry_points
except ImportError:
pass
else:
eps = entry_points()
if not isinstance(eps, dict): # New structure in Python 3.10+
group_eps = (ep for ep in eps if ep.group == group_name)
for entry_point in group_eps:
yield (entry_point.name, entry_point.load)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea – I pushed a commit with a variation on this idea, namely to only even try importlib.metadata on Python 3.10+, when we know it will be stable.

Copy link
Contributor

@podgorniy94 podgorniy94 Jul 19, 2024

Choose a reason for hiding this comment

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

In my initial local implementation, there was a strict check for the Python version, but we decided @j123b567 to abandon this approach since various Python interpreters (e.g., PyPy) and, consequently, different standard modules can be used. My proposed version is universal and not tied to a specific version, but only to the availability of the library if pkg_resources is missing

# "Changed in version 3.10: importlib.metadata is no longer provisional."
try:
from importlib.metadata import entry_points
except ImportError:
Copy link
Member

Choose a reason for hiding this comment

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

Is the try/except block needed if we have an explicit version check? importlib.metadata should always be available on 3.10+

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the try/except block needed if we have an explicit version check? importlib.metadata should always be available on 3.10+

#1102 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

As @podgorniy94 mentioned, I think it's a good thing to double-check, for supporting some more esoteric environments.

Copy link
Contributor

@podgorniy94 podgorniy94 Jul 19, 2024

Choose a reason for hiding this comment

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

As @podgorniy94 mentioned, I think it's a good thing to double-check, for supporting some more esoteric environments.

Actually, there's no need to check the Python version. We simply try pkg_resources, and if the library isn't available, then we attempt importlib.metadata. To avoid strict version checking for, as you said, esoteric environments where there might be different version labeling :) What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and no – using pkg_resources where it's deprecated will raise a DeprecationWarning, which in some (in e.g. my work-work) environments deprecation warnings are hard errors in tests.

@akx akx force-pushed the py312-importlib-metadata branch 2 times, most recently from e012487 to c5434d4 Compare July 19, 2024 12:03
@akx akx self-assigned this Jul 23, 2024
@akx akx requested review from podgorniy94 and tomasr8 July 23, 2024 16:29
@akx akx force-pushed the py312-importlib-metadata branch from c5434d4 to 7a029e0 Compare July 25, 2024 09:39
@akx akx merged commit 4d3fd0e into master Jul 25, 2024
21 of 23 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants