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

Add plotting support for ROIs (Shapes) and Points #14

Closed
9 of 10 tasks
Chris-N-K opened this issue Oct 12, 2022 · 20 comments · Fixed by #21
Closed
9 of 10 tasks

Add plotting support for ROIs (Shapes) and Points #14

Chris-N-K opened this issue Oct 12, 2022 · 20 comments · Fixed by #21
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Chris-N-K
Copy link
Owner

Chris-N-K commented Oct 12, 2022

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:

  • Selection widget for the plotting mode
  • Voxel plotting (shift + mouse move) will stay unchanged
  • ROI plotting, using a Shapes layer for ROI definition
  • Points plotting, using a Points layer for voxel definition

Open Questions:

  1. Where to place the mode selection widget?
  2. Allow Voxel plotting always?
  3. 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)?

TODO:

  • Tests for new functionalities
  • Data extraction function for shapes
  • Rework draw function (add mode support)
  • Add mode to options (OptionsManager, Plotter)
  • Add mode selection widget
  • Readme updates
  • Selectable combination methods for ROI plotting (matching title)
  • Option to modify the title and axe names
  • Scaling factor for the time axis
  • Final checks
@Chris-N-K Chris-N-K self-assigned this Oct 12, 2022
@Chris-N-K Chris-N-K added the enhancement New feature or request label Oct 12, 2022
@Chris-N-K
Copy link
Owner Author

Ideas for the open Questions:

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.

@Chris-N-K
Copy link
Owner Author

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?

@rjlopez2
Copy link
Contributor

Hey thanks.
I am away for the weekend but will check it out as soon as I am back.
Cheers

@rjlopez2
Copy link
Contributor

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:

  • launch napari and napari-time_series_plotter plugin
  • create a synthetic t-2d image
  • select in the options tab the Ploting mode "Shapes"
  • go to the ROI selection layer and draw any shape.

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

@Chris-N-K
Copy link
Owner Author

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

@Chris-N-K
Copy link
Owner Author

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.

@rjlopez2
Copy link
Contributor

Hey Chris,

yes, I just tested, works fine for my dataset and selection layer got locked too
nice 👍🏿

cheers

@Chris-N-K
Copy link
Owner Author

I noticed some annoying things.

  1. 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.

  2. 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.

@rjlopez2
Copy link
Contributor

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?

@Chris-N-K
Copy link
Owner Author

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

@rjlopez2
Copy link
Contributor

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.

@Chris-N-K
Copy link
Owner Author

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

@rjlopez2
Copy link
Contributor

rjlopez2 commented Nov 10, 2022

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:

  • 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 ?
  • 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).

I may think on some other features but from now that's something that just came to my mind while testing it.

cheers.

Ruben

@rjlopez2
Copy link
Contributor

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?

@Chris-N-K
Copy link
Owner Author

* 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.

@rjlopez2
Copy link
Contributor

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

@Chris-N-K Chris-N-K added this to the Version 0.1.0 milestone Nov 14, 2022
@Chris-N-K
Copy link
Owner Author

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

@rjlopez2
Copy link
Contributor

rjlopez2 commented Nov 14, 2022

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):

  • the suggestion: earlier we discussed that may be would be useful to bring the plot option widget to the main canvas so it is easier for the user to interact with the plotting options. Do you still think this would be a good idea? I am in favor of it, but no sure how it will impact on the layout design that still looks pleasant (and obviously how much extra work that would be)..
  • the bug: (this I actually I found it earlier last week but forgot to mention in our last chat) when you input decimals to customize the y/x axis this thow an error I presume because is accepting only integers. I'll try to fix it and seems to be ok now (jut send a pull request) but you can have a look as well.

Beside those, I think it is pretty fine now :)

cheers

Ruben

@Chris-N-K
Copy link
Owner Author

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.

@Chris-N-K Chris-N-K linked a pull request Nov 16, 2022 that will close this issue
@rjlopez2
Copy link
Contributor

fair point
then it should be pretty much done by now.
looking forward for the merge to main :)

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 a pull request may close this issue.

2 participants