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

MNT: drop runtime dependency on pkg_resources (setuptools) #2365

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

neutrinoceros
Copy link
Contributor

@neutrinoceros neutrinoceros commented Feb 19, 2023

Description

resolves #2364

I left one call to pkg_resources.get_distribution in doc/conf because I don't know how to replace it without requiring glue be installed to build docs, and anyway it's not an issue for downstream code.

@neutrinoceros neutrinoceros force-pushed the drop_pkg_resources branch 6 times, most recently from ac9ff87 to ef7cc40 Compare February 19, 2023 18:30
@@ -8,7 +8,6 @@ classifiers =
Operating System :: OS Independent
Programming Language :: Python
Programming Language :: Python :: 3
Programming Language :: Python :: 3.7
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw python_requires = >=3.8 a few lines down, which is why I'm cleaning this marker while I'm in there

@neutrinoceros
Copy link
Contributor Author

I think all failures are unrelated and were already seen in previously merged PRs, so I'll open this for review

@neutrinoceros neutrinoceros marked this pull request as ready for review February 19, 2023 21:55
@neutrinoceros
Copy link
Contributor Author

@astrofrog, anything else I can do here ?

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@astrofrog astrofrog merged commit 9aeb8d4 into glue-viz:main Mar 3, 2023
@neutrinoceros neutrinoceros deleted the drop_pkg_resources branch March 3, 2023 12:12
@dhomeier
Copy link
Collaborator

@neutrinoceros this PR has introduced a failure on test_optional_dependency_not_imported in all py38 and py39 envs, e.g.
https://github.com/glue-viz/glue/actions/runs/4217654549/jobs/7321596947#step:9:485

Unfortunately this was missed among all the very verbose python_exporter failures that are still under review.
I cannot reproduce this with Python 3.9.15 on macOS with the same optional packages installed, but I don't see the ImportError from the test raised on any of the packages in question, but instead an

AttributeError: 'EntryPoint' object has no attribute 'dist'

Yet the test is passing locally with any of setuptools 66.0.0, 67.4.0 or 67.6.0. Any ideas?

@neutrinoceros
Copy link
Contributor Author

sorry about that, and thank you for pointing it out. I will dig into this !

@neutrinoceros
Copy link
Contributor Author

I am able to reproduce the error locally (on Python 3.9.15), however I don't yet understand what is happening. Here are my notes at this point

The dist attribute is still defined at the class level as of importlib_metadata==6.1.0, which is the version you currently get when installing glue in a fresh env with Python 3.9.
https://github.com/python/importlib_metadata/blob/700f2c7d74543e3695163d5487155b92e6f04d65/importlib_metadata/__init__.py#L198

Python 3.11 ships with importlib_metadata==4.13.

@dhomeier
Copy link
Collaborator

The dist attribute is still defined at the class level as of importlib_metadata==6.1.0, which is the version you currently get when installing glue in a fresh env with Python 3.9.

Thanks; I still don't see the error locally after updating to either importlib_metadata==6.0.0 or importlib_metadata==6.1.0 from 5.1.0...

@neutrinoceros
Copy link
Contributor Author

I had it backwards ! So I assumed that importlib.metadata was compatible with the code I wrote no matter what version I used, however this is not quite true. The EntryPoint.dist attribute is only available in importlib_metadata>=3.3, and Python 3.8 ships importlib_metadata==1.3 (I have no information on what version Python 3.9 ships but it seems to be <3.3). Now that I got it, I can patch this.

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.

BUG: importing glue with setuptools>=67.3 raises a DeprecationWarning
3 participants