-
Notifications
You must be signed in to change notification settings - Fork 50
Figure_shape in imagewidget and related histogram widget bug fix #555
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
Conversation
…pe logic in imagewidget to deal with multiple shape values in constructor
Thanks! Is the histogram calc the only thing that was causing issues on your end? |
@kushalkolar I think one thing to note is that the histogram computation right now is nice if the user's data is loaded into RAM. If otoh the user is using ImageWidge to visualize on-the-fly motion correction, things get trickier because the image array object is doing the extra work of loading from disk. In this case, the histogram computations will involve a lot of extra work (loading a large chunk of data into memory just to estimate a histogram of the data). Maybe in that case the user should be able to just specify "absolute" vmin and vmax values without a histogram. I find that knowing the range of the data (via some scale bars) is very useful, even if there is no histogram. |
Ah so you mean like the lazy arrays you have? What about allowing What about data that can be lazy loaded, such as memmaps? We only use a max of 500 x 100 x 100 pixels for computing the histogram, data is subsampled to meet that max number of pixels: We could benchmark this. If we add a kwarg to pass in a pre-computed histogram, we can just pass an "empty" histogram with all zero bins, but have a vmin and vmax so that it can be interacted with. Or a |
is this good to go? LGTM. The other points, such as just operating as a vmin-vmax widget can be done in a later PR |
@kushalkolar Missed your comment about the vmin/vmax. Strongly agree we need something like that -- I will submit a pull for it. |
closes #553, closes #554