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

Import all modules as lazily as possible #319

Merged
merged 5 commits into from
May 4, 2024
Merged

Import all modules as lazily as possible #319

merged 5 commits into from
May 4, 2024

Conversation

toni-neurosc
Copy link
Collaborator

What says in the title, a bunch of changes to delay loading of modules, either submodules of PyNM or 3rd party modules, to the last moment possible right before they are needed.

Result: Package now takes 1.32 seconds to import as opposed to the 4 to 5 seconds it used to take

from time import time

start = time()
import py_neuromodulation
print("Total time sequential:", time() - start, "seconds.")

Total time sequential: 1.321627140045166 seconds.

@toni-neurosc
Copy link
Collaborator Author

Are you ok with merging this too @timonmerk ?

Copy link
Contributor

@timonmerk timonmerk left a comment

Choose a reason for hiding this comment

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

Thanks a lot @toni-neurosc! Those changes are great. It makes sense to change s for settings. The "indirect" import within the functions are also fine. Overall it might reduce a bit readibility of the code since they're not anymore in the same upper part of the file but the performance time improvement is more important imho.

Feel free to merge

@toni-neurosc
Copy link
Collaborator Author

Hi @timonmerk, I thought about the readability issue, it's true that not having imports at the top can make it a bit harder to distinguish dependencies. There is always the possibility of doing Ctrl+F for "import" of course, but apart from that there are 2 solutions I can think of:

  • Keep the imports at the top as comments: this is not particularly pretty either, takes time and it brings the risk of actual imports being changed without the comments being updated... the solution for that newly created problem would be to create a git-hook that runs on commits, or on PR, and that basically looks for imports that are not at the top of the file and adds them as comments to the top of the file specifying where on the file they are (line number, function, class). This would be however a bit laborious and it would take a bunch of hours to get right.

  • Use the LazyLoader function from importlib.utils: https://docs.python.org/3/library/importlib.html#importlib.util.LazyLoader This is the closest to an official lazy loading implementation that exists in Python. I haven't tested it yet, but I can give it a go if you thikn it's a better idea. It would look something like this

from py_neuromodulation.nm_utils import lazy_import

joblib = lazy_import("joblib")

The problem with this is that I have no idea how to implement from joblib import Parallel for example, so it does not cover all of our use cases currently.

There was an official proposal for actual lazy loading in Python but it got rejected sadly https://peps.python.org/pep-0690/

@toni-neurosc toni-neurosc merged commit feb3393 into main May 4, 2024
6 checks passed
@toni-neurosc toni-neurosc deleted the lazy_import branch May 4, 2024 09:18
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