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

ENH: implement lazy imports for optional dependencies #250

Merged
merged 3 commits into from
Sep 15, 2022

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Jul 16, 2022

This is another step to resolve #27
Overall, this gets us to a state where core dependencies (numpy + sympy) are responsible for at least 50-60 % of unyt's import time.

This refactor is similar to my work in yt yt-project/yt#3935
It allows for optional dependencies to never be imported before they are actually requested by users, reducing unyt's own startup time when heavy optional dependencies are installed.

Making matplotlib a true "on-demand" import took a little more work, but now import unyt doesn't cause matplotlib to be imported too when available, reducing the startup time by about 15% to 25% with respect to unyt 2.9.2 (on my machine).

@neutrinoceros neutrinoceros marked this pull request as ready for review July 16, 2022 10:31
@neutrinoceros neutrinoceros changed the title ENH: implement truly *on-demand* imports for optional dependencies ENH: implement lazy imports for optional dependencies Jul 17, 2022
@neutrinoceros neutrinoceros added the enhancement New feature or request label Jul 19, 2022
@neutrinoceros neutrinoceros mentioned this pull request Jul 28, 2022
6 tasks
@neutrinoceros
Copy link
Member Author

this now includes changes for dask, recently added with #185 (ping @chrishavlin)

@neutrinoceros
Copy link
Member Author

this is now based on #272

@neutrinoceros
Copy link
Member Author

@jzuhone @chrishavlin can I request your reviews on this ?

@chrishavlin chrishavlin self-requested a review September 13, 2022 14:34
Copy link
Contributor

@jzuhone jzuhone left a comment

Choose a reason for hiding this comment

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

Curious about the doctests bit, but otherwise looks good.

tox.ini Outdated Show resolved Hide resolved
Copy link
Contributor

@chrishavlin chrishavlin left a comment

Choose a reason for hiding this comment

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

Looks good! Left a couple minor comments.

One additional question -- why'd you put the unyt_arrayConverter class in _mpl_array_converter.__init__.py instead of a new module, _mpl_array_converter.py ?

unyt/_on_demand_imports.py Outdated Show resolved Hide resolved
unyt/_on_demand_imports.py Outdated Show resolved Hide resolved
@neutrinoceros
Copy link
Member Author

neutrinoceros commented Sep 15, 2022

One additional question -- why'd you put the unyt_arrayConverter class in _mpl_array_converter.init.py instead of a new module, _mpl_array_converter.py ?

I can't remember. I would guess I went through several iterations and that form was the first one with which I managed to pass tests, for some reason. Now I agree that a module is better than a mono-module package, and I've checked that it does not in fact break anything, so let's roll with that.

update: it seems that the simpler form breaks on Python 3.8 (see https://github.com/yt-project/unyt/pull/250/checks), so I'll revert this change

@jzuhone jzuhone merged commit 2ee2f5a into yt-project:master Sep 15, 2022
@neutrinoceros neutrinoceros deleted the mpl_optional_import branch September 15, 2022 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize importing unyt
3 participants