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

Implement lazy module importing for all public submodules #2816

Merged
merged 4 commits into from Jun 2, 2021

Conversation

jni
Copy link
Member

@jni jni commented Jun 2, 2021

Description

Fixes #2810

Many thanks to @stefanv and scikit-image/scikit-image#5101 for the precedent.

@tlambert03
Copy link
Member

tlambert03 commented Jun 2, 2021

you'll need to add back all of the non-module exports too, like napari.run, view_image, etc...

@jni
Copy link
Member Author

jni commented Jun 2, 2021

Gah, fix incoming. LOL

Copy link
Member

@tlambert03 tlambert03 left a comment

Choose a reason for hiding this comment

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

lovely! thanks

# to be imported early.
# see: https://github.com/napari/napari/issues/925
# see: https://github.com/napari/napari/issues/1347
from scipy import stats # noqa: F401
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this about?

Copy link
Member Author

Choose a reason for hiding this comment

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

Read the comments just above it. It's weird. 😂

Copy link
Member

Choose a reason for hiding this comment

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

yeah, we honestly still don't know... the relationship with scipy.stats was first observed in #925 (comment)... tried to take it out in #1250, and then #1347 popped up again. would be thrilled if you had any insighty

@stefanv
Copy link
Contributor

stefanv commented Jun 2, 2021

Please keep an eye on the skimage PR; I may still improve that implementation, but it should then be as simple as copying the file here.

@sofroniewn sofroniewn added this to the 0.4.9 milestone Jun 2, 2021
@sofroniewn sofroniewn added the bug Something isn't working label Jun 2, 2021
Copy link
Contributor

@sofroniewn sofroniewn left a comment

Choose a reason for hiding this comment

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

LGTM! Can merge after all tests pass, thanks all!!

@codecov
Copy link

codecov bot commented Jun 2, 2021

Codecov Report

Merging #2816 (c2ada98) into master (9d5021c) will decrease coverage by 0.00%.
The diff coverage is 82.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2816      +/-   ##
==========================================
- Coverage   81.67%   81.67%   -0.01%     
==========================================
  Files         493      494       +1     
  Lines       41280    41295      +15     
==========================================
+ Hits        33717    33727      +10     
- Misses       7563     7568       +5     
Impacted Files Coverage Δ
napari/_lazy.py 77.77% <77.77%> (ø)
napari/__init__.py 83.33% <100.00%> (-4.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d5021c...c2ada98. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

napari.layers cannot be found on master
4 participants