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

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
Contributor

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
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+

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
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