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

DM-39402: Refactor python package determination to use a hierarchy #159

Merged
merged 6 commits into from Jun 1, 2023

Conversation

timj
Copy link
Member

@timj timj commented May 30, 2023

Using importlib.metadata.version can be slow so using it for "a.b.c" and "a.b.c.d1" and "a.b.c.d2" when we know the version of "a.b.c" is wasteful. Use the hierarchy to minimize the number of version requests. Also skip standard library modules.

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

The smptd package is deprecated and will be removed in python
3.12. We don't use chunk anywhere so use that for the import test
instead.
@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Patch coverage: 98.00% and project coverage change: +0.08 🎉

Comparison is base (a34f01d) 93.80% compared to head (7c21978) 93.89%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   93.80%   93.89%   +0.08%     
==========================================
  Files          43       43              
  Lines        2775     2799      +24     
==========================================
+ Hits         2603     2628      +25     
+ Misses        172      171       -1     
Impacted Files Coverage Δ
python/lsst/utils/packages.py 89.89% <97.56%> (+1.63%) ⬆️
tests/test_packages.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

python/lsst/utils/packages.py Outdated Show resolved Hide resolved
python/lsst/utils/packages.py Outdated Show resolved Hide resolved
timj added 5 commits May 31, 2023 10:48
* Skip python standard library and assign them python version.
  They do not have individual package versions and there is no
  point in asking.
* Use a hierarchical approach to version determination. Assume
  that if a.b.c has a version that there is no need to look for
  a.b.c.d. Also assume any component in the hierarchy that starts
  with an underscore is irrelevant since that is a private import.

These changes can make version determination an order of magnitude
faster.
3.8 and 3.9 do not know the stdlib packages and so by
default will not get version numbers for them. This
means that "chunk" can not be used as a test that
new packages turn up after importing. Instead we
fall back to using "smtpd" which works on 3.8
because it has a version string even though most
stdlib packages do not.
_version is filtered out and .version will likely never be
encountered since module "x" should return a version without
needing "x.version".
Instead of creating a hierarchical dict and then recursing
through the dict, take the simpler approach of sorting the modules
names and then comparing the previous line's version with the
current line's hierarchy.
Copy link
Contributor

@ktlim ktlim left a comment

Choose a reason for hiding this comment

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

Looks great.

@timj timj merged commit 5c40a46 into main Jun 1, 2023
13 checks passed
@timj timj deleted the tickets/DM-39402 branch June 1, 2023 04:58
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.

None yet

2 participants