ome / omero-iviewer

An OMERO.web app allowing to view images
https://www.openmicroscopy.org/omero/iviewer/
Other
18 stars 29 forks source link

ROI Color Palette #415

Closed barrettMCW closed 2 years ago

barrettMCW commented 2 years ago

I finally got around to fixing this up. I did a fair bit of testing this time, and as far as I can tell it works as intended.

EDIT (Will): testing: the following config is now on merge-ci and this PR should be included, so the ROI color-picker should correspond to those colors:

omero config set omero.web.iviewer.roi_color_palette "[rgb(0,255,0),rgb(100,50,150)],[darkred,red,pink],[#0000FF, #ff99ff]"
omero config set omero.web.iviewer.show_palette_only true
Screenshot 2022-08-02 at 22 31 20
jburel commented 2 years ago
flake8.main.application   MainProcess    760 INFO     Found a total of 13 violations and reported 1
/omero-iviewer/plugin/omero_iviewer/iviewer_settings.py:43:1: W293 blank line contains whitespace
will-moore commented 2 years ago

I tried the suggested setting from the README:

$ omero config set omero.web.iviewer.roi_color_palette [rgb(0,0,0),blue],[#000000]
bash: syntax error near unexpected token `('

Then with double quotes:

$ omero config set omero.web.iviewer.roi_color_palette "[rgb(0,0,0),blue],[#000000]"

But this doesn't seem to be parsed correctly (just gives 2 black options + plus the default/current yellow):

Screenshot 2022-07-21 at 14 48 16

I wonder if it would be simpler to use json instead of string for the config value. E,g. https://github.com/ome/omero-web/blob/a34be15e664251bfe5decc1aa8f15a7f4801661b/omeroweb/settings.py#L994

Then setting would be like this (and the parsing happens when you start omero-web):

$ omero config set omero.web.iviewer.roi_color_palette '[["rgb(0,0,0)","blue"],["#000000"]]'
barrettMCW commented 2 years ago

That would be mostly correct actually. The yellow shouldn't be there, that must have broke when I changed variable names, I'm looking at it now. But with the config you set both the first items are black. The blue isn't displaying because it's the second item, you'd need to show_palette_only to see the blue. I'll fix the yellow and take another shot at the readme. (I tried to use json, but couldn't get it to work no matter what I did because of some type funkyness)

will-moore commented 2 years ago

I tried using $ omero config set omero.web.iviewer.roi_color_palette "[rgb(0,255,0),rgb(100,50,150)],[darkred,red,pink],[#0000FF, #ff99ff]" but the first row got treated as a single string rgb(0,255,0),rgb(100,50,150)

After a bit of testing, it looks like it fails if you have more than 1 rgb value on any row.

barrettMCW commented 2 years ago

Apologies, missed the console.log and I'm bad with words. Good catch, I fixed the parsing regex. Thanks for your help

will-moore commented 2 years ago
omero.web.iviewer.roi_color_palette=[rgb(0,255,0)],[rgb(0,50,150),darkred,rgb(110,150,0),red],[#0000FF, #ff99ff, rgb(100,50,150)]
omero.web.iviewer.show_palette_only=true

It's looking good now, thanks:

Screenshot 2022-08-01 at 16 57 41

All my testing has been local so far. I'll give it a final go on the merge-ci server tomorrow...

pwalczysko commented 2 years ago

Works fine on merge-ci. Like the intuitiveness of the feature. LGTM

jburel commented 2 years ago

As discussed this morning, it will be good to test the code without the configuration described above to see if there is any side effects.

will-moore commented 2 years ago

@jburel I removed the config on merge-ci (for tomorrow), so if you want to test this PR with config, it would be best to do it today.

jburel commented 2 years ago

Tested without the setting and the viewer works as expected