-
Notifications
You must be signed in to change notification settings - Fork 438
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
Conversation
055e115
to
6017ad0
Compare
6017ad0
to
6395653
Compare
Codecov ReportAttention: Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
babel/messages/_compat.py
Outdated
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)) |
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.
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.
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.
Could you post this as a code suggestion so it's easier to see what you mean?
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.
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) |
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.
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.
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.
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: |
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.
Is the try/except block needed if we have an explicit version check? importlib.metadata
should always be available on 3.10+
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.
Is the try/except block needed if we have an explicit version check?
importlib.metadata
should always be available on 3.10+
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.
As @podgorniy94 mentioned, I think it's a good thing to double-check, for supporting some more esoteric environments.
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.
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?
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.
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.
e012487
to
c5434d4
Compare
For Python 3.12 compatibility! Co-authored-by: podgorniy94 <podgorniy.inc@gmail.com>
c5434d4
to
7a029e0
Compare
Closes #1093 (supersedes it, based on it).
Refs #861.
This PR makes Babel attempt to use
importlib.metadata
instead ofpkg_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.