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

Defer loading element data until attribute access #121

Merged
merged 4 commits into from
May 5, 2024

Conversation

paulromano
Copy link
Collaborator

@paulromano paulromano commented Jun 29, 2023

Here's my quick attempt to reduce the overall import time by deferring the autoload of element data until one actually accesses the corresponding attribute from the mendeleev module, e.g. mendeleev.Li. This seems to work pretty well in my limited testing. The only downside I can see is that the attribute names (H, He, etc.) won't show up in an autocomplete from an IDE. @lmmentel Let me know what you think of this approach.

Fixes #118

Import timing master branch (python3.12):

> time python -c "import mendeleev"
python -c "import mendeleev"  3,78s user 0,78s system 125% cpu 3,624 total
python -c "import mendeleev"  3,67s user 0,85s system 125% cpu 3,588 total
python -c "import mendeleev"  3,86s user 0,85s system 124% cpu 3,782 total

Import timing current branch (python3.12):

> time python -c "import mendeleev"
python -c "import mendeleev"  0,70s user 0,75s system 278% cpu 0,522 total
python -c "import mendeleev"  0,64s user 0,80s system 280% cpu 0,516 total
python -c "import mendeleev"  0,68s user 0,78s system 278% cpu 0,524 total

On average this amounts to 7X speedup of import time.

@kalvdans
Copy link
Collaborator

I rather see a deprecation of these, and force people that want direct access to use mendeleev.elements.H instead of just mendeleev.H.

@lmmentel
Copy link
Owner

lmmentel commented Jun 29, 2023

Nice idea with the lazy loading, does it only work with dot access or also when importing with from mendeleev import Li ?

If it's the latter I think I would rather allow for sth like:

from mendeleev.elements import H, O, Fe

or

from mendeleev.elements import *

Few more places worth updating:

  • update docs on data access
  • ideally we would also have an deprecation warning when someone tries from mendeleev import H

@paulromano
Copy link
Collaborator Author

The lazy loading works with either dot access or the from mendeleev import ... form. I'll go ahead and update this to require the import specifically from mendeleev.elements and update docs/tests accordingly.

@lmmentel lmmentel self-requested a review May 5, 2024 16:08
@lmmentel
Copy link
Owner

lmmentel commented May 5, 2024

I added a few adjustments, mostly catching up the latest changes on master and appeasing the linter.

@lmmentel lmmentel merged commit eb69b2f into lmmentel:master May 5, 2024
20 checks passed
@paulromano paulromano deleted the lazy-elements branch May 6, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce import time
3 participants