glue-viz / glue

Linked Data Visualizations Across Multiple Files
http://glueviz.org
Other
742 stars 153 forks source link

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

Closed neutrinoceros closed 1 year ago

neutrinoceros commented 1 year ago

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 commented 1 year ago

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

neutrinoceros commented 1 year ago

@astrofrog, anything else I can do here ?

dhomeier commented 1 year ago

@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 commented 1 year ago

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

neutrinoceros commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.