-
Notifications
You must be signed in to change notification settings - Fork 29
chore: use package_distributions to get top levels
#861
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #861 +/- ##
=======================================
+ Coverage 92.7% 92.9% +0.2%
=======================================
Files 37 39 +2
Lines 992 993 +1
Branches 98 98
=======================================
+ Hits 920 923 +3
+ Misses 56 55 -1
+ Partials 16 15 -1 ☔ View full report in Codecov by Sentry. |
9f741d0 to
0640431
Compare
d3209f4 to
6b3700c
Compare
|
Having some second thoughts on the new mechanism for falling back to guessing the module name from the package name that this PR would introduce. Several packages don't directly expose modules (for instance some CLIs). So trying to apply a fallback if the package made it clear that there is no exposed module at all does not really feel right in the end. There might be a way to keep the behaviour while being able to use This might be controversial, but should we really have a fallback anyway? Nowadays I believe that most packages do provide the metadata we need, and I personally see the fallback mechanism as too magical, knowing that it could even introduce some false positives in really specific cases. Additionally, users can still provide the mapping themselves if needed, for packages that don't have metadata at all. |
Co-authored-by: Florian Maas <[email protected]>
6b3700c to
9262b33
Compare
PR Checklist
docsis updatedDescription of changes
Those changes are extracted from #854, as a first step towards using
importlib.metadata/importlib_metadatato remove some custom code we have, and make "package" <-> "distribution" discovery more precise.Python 3.10 introduced
packages_distributionsthat returns a mapping that maps Python module names to the packages that expose them, by reading fromtop_level.txtlike we do. In Python 3.11, the method became even smarter by falling back to reading fromRECORDwhen notop_level.txtis present, also exactly like we do.Updates in
importlib.metadataare upstreamed fromimportlib_metadata, so by requiring>=4.13on Python < 3.11, we could usepackages_distributionsin all Python versions we support, and also benefit from the improvement made on Python 3.11.There is one functional change in behaviour with the new implementation, which IMO makes sense to have. Currently, if a package has a
RECORDfile, but we don't match any Python module name in the file, we don't use the fallback that assumes the module name is the name with_instead of-. With the changes, we will now use the fall back if there was no match inRECORD.Test plan