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

Move loading data to separate thread to prevent freezing viewer #283

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Czaki
Copy link
Collaborator

@Czaki Czaki commented Aug 7, 2024

As long time freezing viewer is something not nice for user. Even with speedup loading of data, it could take multiple seconds for big data.

This PR moves loading data and creating Layer to a separate thread, keeping the viewer responsible during load time.
It indicates loading using a progress bar above the selection widget.

Copy link
Collaborator

@melonora melonora left a comment

Choose a reason for hiding this comment

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

Really nice @Czaki, I just had a play with it and for me it works well. @LucaMarconato you want to have a look still before merge?

@LucaMarconato
Copy link
Member

Thanks for the PR! Which PRs from napari do I need for this? I am trying this out on a stock napari but I can't see any different as opposed to the old behavior (interface stuck, no loading bar).

It could also be an os specific behavior?

@LucaMarconato
Copy link
Member

Could you record a GIF showing what the interface should look like please?

@Czaki
Copy link
Collaborator Author

Czaki commented Aug 9, 2024

If you are using macos ARM machine with numpy<2, there will be no change because of a bug in the build of numpy wheels for this platform and version.

napari_spatial-2024-08-09_16.20.07.mp4

@LucaMarconato
Copy link
Member

I am indeed using an ARM machine with numpy<2. I'll try the PR with the latest numpy and let you know. Btw, last time I tried the spatialdata framework with numpy 2 (1 month ago), some dependent libraries did not support it yet scverse/spatialdata#602.

@Czaki
Copy link
Collaborator Author

Czaki commented Aug 11, 2024

Instead of using numpy>=2 you may install numpy with pip install numpy==1.26.4 --no-binary numpy
I have improved numpy guard to detect only numpy from pypi wheel.

relevant numpy issue numpy/numpy#21799

@Czaki
Copy link
Collaborator Author

Czaki commented Aug 12, 2024

This should not be merged before final decison about napari/napari#7146.
As this PR will allow to enable thread loading on macOS with proper napari version.

@LucaMarconato
Copy link
Member

Thanks for the info. I have tried using pip install --force-reinstall numpy==1.26.4 --no-binary numpy but I still couldn't see the progressbar. I'll wait until a decision is made in the linked napari PR and then try again, eventually in clean conda envs and with the desired numpy version. Please ping me when it's time for me to try, thanks!

@Czaki
Copy link
Collaborator Author

Czaki commented Aug 12, 2024

I have tried using pip install --force-reinstall numpy==1.26.4 --no-binary numpy but I still couldn't see the progressbar

did you fetch last changes in this PR?

Copy link

codecov bot commented Aug 30, 2024

Codecov Report

Attention: Patch coverage is 87.09677% with 12 lines in your changes missing coverage. Please review.

Project coverage is 66.50%. Comparing base (69105e0) to head (ca326da).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/napari_spatialdata/_sdata_widgets.py 88.57% 8 Missing ⚠️
src/napari_spatialdata/_viewer.py 82.60% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #283      +/-   ##
==========================================
+ Coverage   63.08%   66.50%   +3.42%     
==========================================
  Files          19       19              
  Lines        2636     2753     +117     
==========================================
+ Hits         1663     1831     +168     
+ Misses        973      922      -51     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

3 participants