Closed Chris-N-K closed 1 year ago
Where to place the mode selection widget?
I would suggest at the top or bottom of the plotter for easy and fast access. I think this might be used more often then other options.
Add specific layers for ROI / Points definition when entering the corresponding modes VS. Allow multiple Shapes / Points layers and select via the LayerSelector (would need a rework for better usability)?
I think the approach to directly link the modes to specific layers makes the coding a lot simpler. To define different ROIs for different images it seems the user can use the grid view as shapes outside of the area of the images will not be used (we need to investigate this behaviour a little further).
If we allow multiple Shapes / Points layers it might be an option to ditch the modes
entirely and go a complete layer selection based approach allowing simultaneous plotting from all sources just depending on layer selection. This would require a rework of the LayerSelector to separate Image, Shapes and Points layers for better navigation.
In the future napari might add a working layer linking or hierarchy then a rework of the general selection approach might be a good idea and Shapes and Points layers directly linked to layers might be an option.
I pushed a version with working Shape and Point layer plotting.
Both are updated automatically after changes. Due to a strange behaviour of the Shapes layer events data a shape newly drawn has to be moved once before the ROI plotting works correctly.
@rjlopez2 would you take a look and give feedback?
Hey thanks. I am away for the weekend but will check it out as soon as I am back. Cheers
Hi Chris,
this is looking really nice, many thanks again :)
I don't know if you ever tested with t+2d images but I am receiving an error message after drowing a shape when plotting in "shape" mode.
this is the way to reproduce the issue:
I did it using the following:
viewer2 = napari.Viewer()
viewer2.add_image(np.random.rand(50, 64, 64), name= '3d_image')
this is the full error message:
Traceback (most recent call last): File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/vispy/app/backends/_qt.py", line 502, in mousePressEvent self._vispy_mouse_press( File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/vispy/app/base.py", line 184, in _vispy_mouse_press ev = self._vispy_canvas.events.mouse_press(**kwargs) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/vispy/util/event.py", line 453, in __call__ self._invoke_callback(cb, event) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/vispy/util/event.py", line 471, in _invoke_callback _handle_exception(self.ignore_callback_errors, File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/vispy/util/event.py", line 469, in _invoke_callback cb(event) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/_qt/qt_viewer.py", line 990, in on_mouse_press self._process_mouse_event(mouse_press_callbacks, event) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/_qt/qt_viewer.py", line 949, in _process_mouse_event mouse_callbacks(layer, event) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/utils/interactions.py", line 124, in mouse_press_callbacks next(gen) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/layers/shapes/_shapes_mouse_bindings.py", line 142, in add_rectangle yield from _add_line_rectangle_ellipse( File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/layers/shapes/_shapes_mouse_bindings.py", line 152, in _add_line_rectangle_ellipse layer.add(data, shape_type=shape_type) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/layers/shapes/shapes.py", line 2031, in add self.events.data(value=self.data) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/utils/events/event.py", line 757, in __call__ self._invoke_callback(cb, event if pass_event else None) File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/utils/events/event.py", line 794, in _invoke_callback _handle_exception( File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari/utils/events/event.py", line 781, in _invoke_callback cb(event) File "/Users/rubencito/p_experimental/image_analysis/napari_play/napari-time_series_plotter/src/napari_time_series_plotter/widgets.py", line 211, in _data_changed_callback self._draw() File "/Users/rubencito/miniconda3/envs/omass_env/lib/python3.9/site-packages/napari_matplotlib/base.py", line 119, in _draw self.draw() File "/Users/rubencito/p_experimental/image_analysis/napari_play/napari-time_series_plotter/src/napari_time_series_plotter/widgets.py", line 124, in draw roi_ts = extract_mean_ROI_shape_time_series( File "/Users/rubencito/p_experimental/image_analysis/napari_play/napari-time_series_plotter/src/napari_time_series_plotter/utils.py", line 69, in extract_mean_ROI_shape_time_series return layer.data[mask].reshape(dshape[0], -1).mean(axis=1) IndexError: boolean index did not match indexed array along dimension 1; dimension is 64 but corresponding boolean dimension is 4096
I was receiving this message error when you passed the full dim of the image for the mask instead as just the first dimension as I had it before. See more here. when I reverted to the way it was before, it works. You can of course change the logic if you want, but just like it is right now is not working for t-2d images (at least in my hands).
I tried similarly to plot and test t-3d images and is not showing this error and seems to do the work just as expected. I can confirm this for the 3 plotting modalities.
About the automatic update, thanks, great job! this is very useful feature and it is working just great :)
Now, I think it would be anyway good time to assert a test for this kind of situations and also in general for the the extract_mean_ROI_shape_time_series
function in your test routine. I could help with that but I don't know how to do it yet. I am right now having a look at that.
I also think is good idea to move at least the plotting option widget element to the main plotter tab since it is quite useful to have it right there at hand.
Cheers
Thanks for testing and happy you like it.
Guess I have to take a look at the extraction logic, I thought I tested 2D+t but who knows xD. It's probably just a trivial mix-up... Will push a fix as fast as I can manage.
If you know other people with time resolved images it would be nice if you asked them for suggestions. I would like to engage 0.1.0 in the near future but need more feed back on the plugin in general.
Best Chris
I push the changes I made till now. Could you test if it works for your data again? And could you test if you can delete the selection layer? If everything works properly you should not be able to ^^
If everything works fine I just have to finish the tests and this might be ready for pull.
Hey Chris,
yes, I just tested, works fine for my dataset and selection layer got locked too nice 👍🏿
cheers
I noticed some annoying things.
With the current approach there are some issues if the user wants to define points and shapes on multiple slices. The reason is that the moment an empty points or shapes layer is added one must define its dimension (default 2D). These can not be changed afterwards without replacing the whole data. Thus, we can either add them already in 4D to support 3D and 4D but no nD images, add some more or less complicated functions to automatically alter the selection layer or switch to the selection widget approach. I'm not sure what would be more work the second or third approach. I think for a first release of the general ROI and points plotting functionality I will use the first one.
The ROI plotting via shapes does not support translate, scale, etc. values. This is caused by the to_labels method of the shapes layer. A fix will need some substantial work as I would need to write a custom function to use instead of the to_labels method.
Hi Chris,
no sure if I could completely followed you on the issues you are mentioning here... Just let me know how can I help or be supportive for fixing the problems, e.g testing, etc... In any case I will be super glad to discuss it further may be via email, videocall or something else?
Hi Ruben, was more or less writing this mainly to remember it ^^ You could take a look at the current state of the branch. I brought it to a stage where it should work fine in most cases and updated the Readme. Could you check for typos and comprehensibility? The Readme as well as the in code doc? Would be super helpful! Just make a pull request with corrections if needed to the branch.
Best, Chris
Hey Chris, sorry for my long brake on this front, just finally got time to check this out now. Hey men I have to really say wao! you made a great job on improving the documentation. Many things looks much more clear for me now and hope also for others. Also very nice feature you add on multiples plotting function in roi mode. I was about to suggest it. I also saw you made a number of new test for the many new functionalities. Really nice I just made a pull request for few typos that I found but in general looks really really good men thanks 👍🏿 let me know how I can support any further from here :) Cheers.
Hi, no worries. Thanks for the testing your support is much appreciated. In the mean time I noticed I should add some minor things like a plot title matched to the roi function. I guess the release could probably happen next week.
If you like to support this project in the future feel free to think about more things that might be useful and should be part of the plugin. If you find something just open an issue or a pull request. Otherwise the next big step to 0.1.0 is a tool to save the time series data in csv form. There is already a branch, but I think it's not up to date with my local one. Would appreciate ideas and input. At the moment I believe it's best to extract the plot data from the mpl canvas object.
Best, Chris
You are very welcome, indeed I am learning a lot by contributing in this very nice project :) Actually, while experimenting and playing a bit with it I got couple of ideas that could be potentially usefull, for instance:
I may think on some other features but from now that's something that just came to my mind while testing it.
cheers.
Ruben
I also have been thinking on a potential solution for this:
...the next big step to 0.1.0 is a tool to save the time series data in csv form...
may be implementing something from the napari-region-pros plugin could help?
* for the plotting options on ROI mode, we could create a combobox displaying not just mena, median, and so on.. but actually all the possibilities (or a least many that make sense) from the numpy methods ?
What other functions do you find useful?
* also, when plotting (in the canvas) time axis can compute the actual time by accessing the resolved time metadata when available (for instance frames per second acquisition from your camera/sensor).
This would be really nice but is difficult to realise as it is highly dependant on said metadata. I think an automatic version is not possible with all the ways to load image with and without metadata and no common way of storing said data. The only some what stable solution I can think about would be to use the scaling value of the first axis, but again that is dependant on the way the image was loaded into the viewer. In contrast, an option for a scaling factor in the options tab would be easy to implement. One could implement a field for the title and axe names as well. Guess I could add these to the branch before pulling it to main, or you could if you want to try.
I also have been thinking on a potential solution for this:
...the next big step to 0.1.0 is a tool to save the time series data in csv form...
may be implementing something from the napari-region-pros plugin could help?
Yea, I thought about that too. The question is would it be good to add additional dependencies and does it really help. If you like you could explore this.
What other functions do you find useful?
I was thinking on min and max and then I thought may be that's all LOL ^^ those two should be anyway easy to include....
This would be really nice but is difficult to realise as it is highly dependant on said metadata.
I was thinking exactly on this and researching a bit if there was some kind of "convention" or standard for napari handeling metadata, but could not find something clear. Probably the quicker and simpler solution would be to implement what you just suggested and just add a field where user can write down fps for instance...
Yea, I thought about that too. The question is would it be good to add additional dependencies and does it really help. If you like you could explore this.
Yes, I feel the same. However at some point I think it will require external dependencies like pandas or some kind of tabular data handler for listing the values? I guess if we try borrow their bare logic to create/export the tables and avoid the rest (reg properties analysis, etc...) would be sufficient? I will give it a try anyway as soon as I get a bit of time, since i think many people (including us) will benefit a lot from this feature.
thanks again.
Ruben
Hi @rjlopez2, I pushed the finalised version with added ROI combinations modes Min and Max and options for customising the plot title and the axe labels, as well as a scaling option for the x-axis. Could you take a final look before I pull the changes and start the deployment process please?
Cheers, Chris
Hey Chris, thanks men, I just test it for voxel and shapes mode this is really looking awesome 👌🏿. I asume is the same for points. I have however a suggestion and unfortunately a little bug report (I think super easy to spot):
Beside those, I think it is pretty fine now :)
cheers
Ruben
Thanks for the pull request, merged it should be all fine now. For the design decision of the mode selection, I think for now it will stay the way it is. There are only two other places for the menus and that is either above the navigator bar of the plotter or below the plot. In both cases the structure of keeping the options inside of the options manager will no longer work. Thus, this step should be premeditated. Maybe we will get some more feedback when other people are using the new features.
I will start a pull request for the branch.
fair point then it should be pretty much done by now. looking forward for the merge to main :)
Add functionality to define the data for plotting by either shapes in a Shapes layer or points in a Point layer. This would result in three different plotting modes: Voxel, Shapes and Points
Features:
Open Questions:
TODO: