-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Replace pkg_resources with importlib.metadata #11091
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
b2681bc to
d89c38a
Compare
docs/conf.py
Outdated
|
|
||
| # The full version, including alpha/beta/rc tags. | ||
| release = get_distribution(project).version | ||
| release = version(project) |
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'm going to use this everywhere now!
|
This touches I guess adding it as a required external dependency for just Python 3.7 is acceptable annoyance if it has clear performance gain. |
|
Sorry I'm just noticing this now. |
eslavich
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.
This looks great to me, seems like we should adopt importlib.metadata in asdf as well. Thanks for showing us the way!
|
@saimn Thanks! Feel free to merge after a rebase. |
1a4b37b to
24d649e
Compare
|
👍 🎊 Python packaging has come a long way since this was last worked on. |
|
Just to be sure, can you please rebase? All the CI should be green now. |
|
Also, since this introduces a new required dependency, it needs a change log. Alas, the towncrier check doesn't work right now, so you won't get reminded in the status checks. |
|
FWIW the importlib-metadata dependency is just a backport for Python 3.7, but I agree it merits a changelog mention. |
Pytest is now also using importlib.metadata so I guess this is no more useful.
pllim
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.
LGTM. Thanks!
Ref astropy#11091 (review) packaging is used in tests and the Sphinx config so we should list it explicitly as a dependency, even if it's already a dependency for pytest and Sphinx.
Ref astropy#11091 (review) packaging is used in tests and the Sphinx config so we should list it explicitly as a dependency, even if it's already a dependency for pytest and Sphinx.
Ref astropy#11091 (review) packaging is used in tests and the Sphinx config so we should list it explicitly as a dependency, even if it's already a dependency for pytest and Sphinx.
Following discussions on
pkg_resourcesvsimportlib.metadata:pkg_resourcesis slow (1, 2, 3), which is the reason why importlib.metadata was added to the stdlib in Python 3.8 (with a backport for 3.7).For instance, importing

astropy.modeling:There is a better alternative, so let's use it ?