Simple refactoring to reduce astropy import time. #7647
Conversation
|
Hi there @mhvk 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here. |
astropy/utils/introspection.py
Outdated
| if m: | ||
| version = m.group(0) | ||
| from distutils.version import LooseVersion | ||
| # LooseVersion raises a TypeError when strings like dev, rc1 are part |
There was a problem hiding this comment.
I don't know if this is true anymore:
In [5]: LooseVersion('3.0dev0') < LooseVersion('3.0dev1')
Out[5]: TrueThere was a problem hiding this comment.
Good point. All tests pass on python 3.7 at least without it. I'll push a new commit that removes the doctoring - we'll see if the tests #5944 added pass on all versions.
|
Very nice improvement! |
|
Wow, very nice catch! |
|
OK, tests all passed without the |
| assert not minversion(test_module, version) | ||
|
|
||
|
|
||
| def test_minversion(): |
There was a problem hiding this comment.
Does _minversion_test above still run with this removed?
There was a problem hiding this comment.
I agree, I think _minversion_test needs to be renamed to test_minversion
There was a problem hiding this comment.
Wow, proof yet again how useful review is... How could I have missed that!
mhvk
force-pushed
the
simple-import-time-reduction
branch
from
d686fc0 to
4855071
Compare
|
I'm merging this as it's been reviewed and then approved. Thanks @mhvk! |
Switch to robust version parsing (packaging/pkg_resources) to avoid LooseVersion comparison errors for dev versions. Adds regression test for issue astropy#7647.
4 participants
This does two things: remove the use of
pkg_resourcesinutils.misc.minversionin favour of its backup (distutils.version.LooseVersion), which is much more light-weight; ensureurllibgets imported only when actually needed, at least at import time.Especially the removal of
pkg_resourceshas a large effect; with this PR: