DM-42391: Make use of packages_distributions() in utils.packages #178
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #178 +/- ##
==========================================
- Coverage 94.31% 93.98% -0.34%
==========================================
Files 46 46
Lines 3186 3206 +20
==========================================
+ Hits 3005 3013 +8
- Misses 181 193 +12 ☔ View full report in Codecov by Sentry. |
|
Also on this PR I removed support for python 3.9 (since |
| @@ -49,7 +50,9 @@ | |||
| # Python modules to attempt to load so we can try to get the version | |||
| # We do this because the version only appears to be available from python, | |||
| # but we use the library | |||
| PYTHON = {"galsim"} | |||
| PYTHON: set[str] = set() | |||
There was a problem hiding this comment.
In some sense we should delete all this stuff since it's a bit unreliable and for us conda reports all these versions directly. Not for this ticket of course.
| namespace = name.split(".")[0] | ||
|
|
||
| # package_dist is a mapping from import namespace to distribution | ||
| # package names. This may be a one-to-many mapping due to namespace |
There was a problem hiding this comment.
Such as:
sphinxcontrib: ['sphinxcontrib-bibtex', 'sphinxcontrib-autoprogram', 'sphinxcontrib-htmlhelp', 'sphinxcontrib-qthelp', 'sphinxcontrib-serializinghtml', 'sphinxcontrib-devhelp', 'sphinxcontrib-jsmath', 'sphinxcontrib-applehelp', 'sphinxcontrib-doxylink', 'sphinxcontrib-jquery']
lsst: ['ts-xml']
?
There was a problem hiding this comment.
Exactly. This is somewhat described in https://packaging.python.org/en/latest/guides/packaging-namespace-packages/#packaging-namespace-packages but it's not strictly required but it is the convention for every set that I've seen.
| # Check if the components start with the namespace; this is | ||
| # needed because (at least) lsst.ts packages do not use | ||
| # ``lsst`` in the package name. | ||
| if dist_name_parts[0] != namespace: |
There was a problem hiding this comment.
Do you want to install ts-xml in the action so you can get code coverage? It would be nice if we always had the lsst prefix in the public distributions of things like this...
|
|
||
| if not found: | ||
| # Fall back to name. | ||
| dist_name = name |
There was a problem hiding this comment.
Is this what happens currently in the EUPS case? Maybe mention that in the comment?
…ble. This new version uses importlib.metadata.packages_distributions() to get mappings from namespace to packages installed. There are extra comparisons that are needed to properly handle namespace packages which are still a little janky with this system.
This way we can track that everything works when there is a conda package using the lsst namespace.
erykoff
force-pushed
the
tickets/DM-42391
branch
from
c81901b
to
494e735
Compare
|
Re: |
Checklist
doc/changes