ome / omero-cli-transfer

An OMERO CLI plugin for creating transfer packets between OMERO servers.
https://pypi.org/project/omero-cli-transfer/
GNU General Public License v2.0
14 stars 13 forks source link

Transfer of ROI settings #13

Closed pwalczysko closed 1 year ago

pwalczysko commented 2 years ago

How does the transfer of the ROI settings such as color of the stroke or fill work ?

I have encountered 2 problems:

  1. the stroke color does not get tranferred, the ROI will appear with no color stroke.
  2. there was an error regarding fill color as shown below in one case
``` Matching source and destination images... Creating and linking OMERO objects... Traceback (most recent call last): File "/Users/pwalczysko/miniconda3/envs/cli37/bin/omero", line 11, in sys.exit(main()) File "/Users/pwalczysko/miniconda3/envs/cli37/lib/python3.7/site-packages/omero/main.py", line 125, in main rv = omero.cli.argv() File "/Users/pwalczysko/miniconda3/envs/cli37/lib/python3.7/site-packages/omero/cli.py", line 1784, in argv cli.invoke(args[1:]) File "/Users/pwalczysko/miniconda3/envs/cli37/lib/python3.7/site-packages/omero/cli.py", line 1222, in invoke stop = self.onecmd(line, previous_args) File "/Users/pwalczysko/miniconda3/envs/cli37/lib/python3.7/site-packages/omero/cli.py", line 1299, in onecmd self.execute(line, previous_args) File "/Users/pwalczysko/miniconda3/envs/cli37/lib/python3.7/site-packages/omero/cli.py", line 1381, in execute args.func(args) File "/Users/pwalczysko/Work/omero-cli-transfer/src/omero_cli_transfer.py", line 93, in _wrapper return func(self, *args, **kwargs) File "/Users/pwalczysko/Work/omero-cli-transfer/src/omero_cli_transfer.py", line 134, in unpack self.__unpack(args) File "/Users/pwalczysko/Work/omero-cli-transfer/src/omero_cli_transfer.py", line 213, in __unpack populate_omero(ome, img_map, self.gateway, hash) File "/Users/pwalczysko/Work/omero-cli-transfer/src/generate_omero_objects.py", line 200, in populate_omero create_rois(ome.rois, ome.images, img_map, conn) File "/Users/pwalczysko/Work/omero-cli-transfer/src/generate_omero_objects.py", line 110, in create_rois fill_color = _int_to_rgba(int(roi.union[0].fill_color)) TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType' ```
erickmartins commented 2 years ago

I'll do some testing - I can see problem 2 happening if fill color is completely unset on the origin ROI (in which case we'll probably have to have some sane default), but fill and stroke color should have been transferred.

erickmartins commented 2 years ago

I've just tested with a few ROIs and stroke/fill color were transfered correctly. I wonder if what you're seeing is a consequence of caveat number 7 from our previous repo README:

OMERO does some strange setting with channel = -1 for ROIs sometimes (for RGB images, I think). ome-types hates that, so we set those values to 0.

I don't know exactly how to fix this here - it would either require ome-types to accept channel = -1 for an ROI, or some clever way of bypassing that.

erickmartins commented 2 years ago

(FWIW, I did see this issue pop up in some unrelated testing and I think #15 also fixes it)

pwalczysko commented 1 year ago

I have a new occurrence of this issue with 0.3.1 release

omero transfer unpack dataset-to-plate.tar
...
Plate:42505
Other imported objects:
Fileset:43258

==> Summary
139 files uploaded, 1 fileset, 1 plate created, 16 images imported, 0 errors in 0:01:28.948
Matching source and destination images...
Creating and linking OMERO objects...
id='ROI:501662' annotation_ref=[] description=None name=None union=[Ellipse(
   id='Shape:590991',
   text='117.00000',
   the_c=0,
   the_t=0,
   the_z=0,
   radius_x=6.9782385,
   radius_y=6.9782385,
   x=1951.8583,
   y=382.31174,
)]
None
Traceback (most recent call last):
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/bin/omero", line 8, in <module>
    sys.exit(main())
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero/main.py", line 126, in main
    rv = omero.cli.argv()
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero/cli.py", line 1787, in argv
    cli.invoke(args[1:])
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero/cli.py", line 1225, in invoke
    stop = self.onecmd(line, previous_args)
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero/cli.py", line 1302, in onecmd
    self.execute(line, previous_args)
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero/cli.py", line 1384, in execute
    args.func(args)
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero_cli_transfer.py", line 125, in _wrapper
    return func(self, *args, **kwargs)
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero_cli_transfer.py", line 195, in unpack
    self.__unpack(args)
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/omero_cli_transfer.py", line 347, in __unpack
    hash, folder, self.metadata)
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/generate_omero_objects.py", line 404, in populate_omero
    create_rois(ome.rois, ome.images, img_map, conn)
  File "/Users/pwalczysko/opt/anaconda3/envs/cli-transfer/lib/python3.7/site-packages/generate_omero_objects.py", line 270, in create_rois
    fill_color=fill_color, stroke_color=stroke_color)
UnboundLocalError: local variable 'fill_color' referenced before assignment

Maybe it is a variation - feel free to move the comment or tell me to do so - the ROIs got imported okay, but the error occcurred as above.

erickmartins commented 1 year ago

I think the main issue here is that ezomero Shapes (which we're using to create ROIs) don't have fill_color, stroke_color and stroke_width themselves, and they just use whatever is set at the ROI level. Meanwhile, omero-cli-transfer looks for those parameters at the Shapes, and adds those parameters to them in the metadata/XML file. For now, I will probably implement defaults in case they are not set at ROI level (maybe look at the first Shape and use that), but doing this "right" will probably require some rewriting on the ezomero side as well. I'll open an issue there.

erickmartins commented 1 year ago

PR #37 implements a partial fix for this - if the first Shape in an ROI has these attributes we use them, otherwise we default to (0,0,0,0). A full fix requires a new ezomero version.

erickmartins commented 1 year ago

Release 0.3.2 includes the latest PR. Let me know if it works as intended!

pwalczysko commented 1 year ago

Release 0.3.2 includes the latest PR. Let me know if it works as intended!

Thank you. With 0.3.2 it works without error, but it seems that I am getting more ROIs in the resulting plate image than in the original. The original has 184, the one after unpack has 357 - there are some masks, which did not get duplicated, but the point ROIs did get somehow duplicated (there are two point ROIs on top of each other for each cell now). I will have a look on a creation of a simpler example with just couple of ROIs.

pwalczysko commented 1 year ago

Release 0.3.2 includes the latest PR. Let me know if it works as intended!

Thank you. With 0.3.2 it works without error, but it seems that I am getting more ROIs in the resulting plate image than in the original. The original has 184, the one after unpack has 357 - there are some masks, which did not get duplicated, but the point ROIs did get somehow duplicated (there are two point ROIs on top of each other for each cell now). I will have a look on a creation of a simpler example with just couple of ROIs.

I cannot repeat the doubling problem if there are only point ROIs on the image (i.e. no masks). In such case, the number of ROIs after unpack is matching the original, just that the ROIs after unpack have no color of the stroke it seems (selectable, but invisible stroke, which is possibly expected after your fix?). Any idea why the presence of masks should be important for the doubling bug ?

pwalczysko commented 1 year ago

I think I hit an unrelated issue as well when testing this, see https://github.com/ome/omero-cli-transfer/issues/12#issuecomment-1387326768

erickmartins commented 1 year ago

what exactly do you mean by "masks" here? What kind of masks? Can you send an example?

pwalczysko commented 1 year ago

Hi Erick, I mean a mask in OMERO, something like in https://forum.image.sc/t/prepping-and-including-roi-masks/48750/8 - you can see in that post how these are displayed in OMERO.iviewer. I have a mask on an image (mask is a type of RoI in OMERO) and some other ROIs (polygons). If this is the case, then it seems to me that the pack command is not doing the correct thing. I have two examples, in one,

  1. in one example, the masks are packed, but when the image is unpacked, the polygon number is doubled.
  2. in another example (more worrying), in case there are 3 ROIs, out of which there are two polygons and one mask, then all the ROIs are ignored by pack - if the mask is deleted, and the pack is unleashed on the image with the remaining 2 polygons, then all works as expected (i.e. the unpack is executed, resulting in an image with 2 polygons in OMERO)

The problem is the size of both those examples, one is a plate, and one is a large image.

I will ponder about how to create a smallish example for you.

erickmartins commented 1 year ago

thanks, Petr - we don't use OMERO masks at all, so I don't really have an easy way to generate a quick example for testing. The color/strokewidth issue needed some more work, but my current dev branch sorts that out (and the plate repeat import issue). As soon as I can check (and solve) this duplication problem I'll push a new release.

pwalczysko commented 1 year ago

thanks, Petr - we don't use OMERO masks at all, so I don't really have an easy way to generate a quick example for testing. The color/strokewidth issue needed some more work, but my current dev branch sorts that out (and the plate repeat import issue). As soon as I can check (and solve) this duplication problem I'll push a new release.

Hi Erick, so tried to create a simple image example with masks for you, which succeeded. What did not succeed is to force any tools, such as OMERO.downloader do export that image with the mask. So the only way I see to give you something to reproduce is:

  1. have a simple image in OMERO, I tried just a screenshot png
  2. Run against that image the script below [1], replacing the HOST, USERNAME, PWD, PORT and ImageID in that script
  3. Observe that you have 5 ROIs after that on that image in OMERO, one of which is a mask

Screenshot 2023-01-23 at 13 41 14

  1. Try to pack that image, and unpack -> Observe that you have only 4 ROIs, the mask is missing

Screenshot 2023-01-23 at 13 39 50

[1] - script for mask creation

``` #!/usr/bin/env python # -*- coding: utf-8 -*- # # Copyright (C) 2014 University of Dundee & Open Microscopy Environment. # All Rights Reserved. # Use is subject to license terms supplied in LICENSE.txt # """ FOR TRAINING PURPOSES ONLY! """ from __future__ import division from __future__ import print_function from future.utils import native_str from builtins import str from builtins import range from past.utils import old_div import numpy import struct import math import omero from omero.model.enums import UnitsLength from omero.rtypes import rdouble, rint, rstring from omero.gateway import BlitzGateway from omero.gateway import ColorHolder imageId = int($REPLACE) """ start-code """ # Create a connection # =================== USERNAME = $REPLACE PASSWORD = $REPLACE HOST = $REPLACE PORT = $REPLACE conn = BlitzGateway(USERNAME, PASSWORD, host=HOST, port=PORT) conn.connect() updateService = conn.getUpdateService() # Create ROI # ========== # We are using the core Python API and omero.model objects here, since ROIs # are not yet supported in the Python Blitz Gateway. # # First we load our image and pick some parameters for shapes x = 50 y = 200 width = 100 height = 50 image = conn.getObject("Image", imageId) z = old_div(image.getSizeZ(), 2) t = 0 # We have a helper function for creating an ROI and linking it to new shapes def create_roi(img, shapes): # create an ROI, link it to Image roi = omero.model.RoiI() # use the omero.model.ImageI that underlies the 'image' wrapper roi.setImage(img._obj) for shape in shapes: roi.addShape(shape) # Save the ROI (saves any linked shapes too) return updateService.saveAndReturnObject(roi) # Another helper for generating the color integers for shapes def rgba_to_int(red, green, blue, alpha=255): """Return the color as an Integer in RGBA encoding.""" return int.from_bytes([red, green, blue, alpha], byteorder='big', signed=True) # create a Rectangle shape (added to ROI below) print("Adding a rectangle at theZ: %s, theT: %s, X: %s, Y: %s, width: %s," " height: %s" % (z, t, x, y, width, height)) rect = omero.model.RectangleI() rect.x = rdouble(x) rect.y = rdouble(y) rect.width = rdouble(width) rect.height = rdouble(height) rect.theZ = rint(z) rect.theT = rint(t) rect.textValue = rstring("test-Rectangle") rect.fillColor = rint(rgba_to_int(255, 255, 255, 255)) rect.strokeColor = rint(rgba_to_int(255, 255, 0, 255)) # create an Ellipse shape (added to ROI below) ellipse = omero.model.EllipseI() ellipse.x = rdouble(y) ellipse.y = rdouble(x) ellipse.radiusX = rdouble(width) ellipse.radiusY = rdouble(height) ellipse.theZ = rint(z) ellipse.theT = rint(t) ellipse.textValue = rstring("test-Ellipse") # Create an ROI containing 2 shapes on same plane # NB: OMERO.insight client doesn't support display # of multiple shapes on a single plane. # Therefore the ellipse is removed later (see below) create_roi(image, [rect, ellipse]) # create an ROI with single line shape line = omero.model.LineI() line.x1 = rdouble(x) line.x2 = rdouble(x+width) line.y1 = rdouble(y) line.y2 = rdouble(y+height) line.theZ = rint(z) line.theT = rint(t) line.textValue = rstring("test-Line") create_roi(image, [line]) def create_mask(mask_bytes, bytes_per_pixel=1): if bytes_per_pixel == 2: divider = 16.0 format_string = "H" # Unsigned short byte_factor = 0.5 elif bytes_per_pixel == 1: divider = 8.0 format_string = "B" # Unsigned char byte_factor = 1 else: message = "Format %s not supported" raise ValueError(message) steps = math.ceil(old_div(len(mask_bytes), divider)) mask = [] for i in range(int(steps)): binary = mask_bytes[ i * int(divider):i * int(divider) + int(divider)] format = native_str(int(byte_factor * len(binary))) + format_string binary = struct.unpack(format, binary) s = "" for bit in binary: s += str(bit) mask.append(int(s, 2)) return bytearray(mask) mask_x = 50 mask_y = 50 mask_h = 100 mask_w = 100 # Create [0, 1] mask mask_array = numpy.fromfunction( lambda x, y: (x * y) % 2, (mask_w, mask_h)) # Set correct number of bytes per value mask_array = mask_array.astype(numpy.uint8) # Convert the mask to bytes mask_array = mask_array.tostring() # Pack the bytes to a bit mask mask_packed = create_mask(mask_array, 1) # Define mask's fill color mask_color = ColorHolder() mask_color.setRed(255) mask_color.setBlue(0) mask_color.setGreen(0) mask_color.setAlpha(100) # create an ROI with a single mask mask = omero.model.MaskI() mask.setTheC(rint(0)) mask.setTheZ(rint(0)) mask.setTheT(rint(0)) mask.setX(rdouble(mask_x)) mask.setY(rdouble(mask_y)) mask.setWidth(rdouble(mask_w)) mask.setHeight(rdouble(mask_h)) mask.setFillColor(rint(mask_color.getInt())) mask.setTextValue(rstring("test-Mask")) mask.setBytes(mask_packed) create_roi(image, [mask]) # create an ROI with single point shape point = omero.model.PointI() point.x = rdouble(x) point.y = rdouble(y) point.theZ = rint(z) point.theT = rint(t) point.textValue = rstring("test-Point") create_roi(image, [point]) # create an ROI with a single polygon, setting colors and lineWidth polygon = omero.model.PolygonI() polygon.theZ = rint(z) polygon.theT = rint(t) polygon.fillColor = rint(rgba_to_int(255, 0, 255, 50)) polygon.strokeColor = rint(rgba_to_int(255, 255, 0)) polygon.strokeWidth = omero.model.LengthI(10, UnitsLength.PIXEL) points = "10,20 50,150 200,200 250,75" polygon.points = rstring(points) polygon.textValue = rstring("test-Polygon") create_roi(image, [polygon]) conn.close() ```
erickmartins commented 1 year ago

I'll give it a try - thanks! I'll say that right now this looks like it's behaving as intended, we do not have Masks implemented. It's something we can discuss for the future, but I'd expect only 4 ROIs in this case. Is this not causing the duplication issue you've seen before?

pwalczysko commented 1 year ago

Is this not causing the duplication issue you've seen before?

Not with this simple example, I actually have 2 issues, a duplication one, and a loss of all ROIs is another. Will consult about how to get that to you maybe...

pwalczysko commented 1 year ago

So, with regards to the duplication issue, we debugged it with @will-moore a bit and think we have a culprit. The issue is as follows

  1. Have an image file with ROIs included in the file, such as https://downloads.openmicroscopy.org/images/Leica-LIF/seanwarren/150519_FRAP_test_ROIs_chromagreen/150519_FRAP_test_ROIs_chromagreen.lif
  2. Import that file into OMERO, it will have automatically some ROIs because of Ad. 1 above
  3. Delete the Text ROI -> this is necessary, because there is a separate problem with Text ROIs and omero-cli-transfer
  4. Now you have just 1 polygon ROI per image
  5. pack and unpack the images
  6. Observe that the newly unpacked images have now 3 ROIs - 1 original polygon, 1 original Text ROI, 1 new polygon.

This is because the pack and unpack workflow added the ROI which was in OMERO, but also the import imported the ROIs which were already in the original file.

How to solve: There would be couple of suggestions, such as deleting the ROIs coming from the original file right after the import step of the unpack command, then only add the new ROIs which were transferred from OMERO during pack. But there might be other ways to go about this.

Sorry about the masks, this was a complete Red Herring, I was just confused by my own testing file which had ROIs from original file some of which were masks... - so a new issue, without masks involvement now :)

erickmartins commented 1 year ago

Alright - this makes a lot of sense actually. I think deleting file ROIs after import is probably the best choice here; it respects the user changes in ROIs on the source side and should provide the one-to-one correspondence we're striving for with this library. Thanks for the investigation, once again!

pwalczysko commented 1 year ago

Sorry to add to this pile, but actually I think it belongs to here:

I think that the fill color is not being transferred at all, in fact, my new ROIs after unpack have no fill at all, although the original ones had nice fill from QuPath, see e.g. https://omero-guides.readthedocs.io/en/latest/qupath/docs/qupath.html - is it expected ? Certainly, the colors of the ROIs in this case signifies the Annotation type, and it would make sense to preserve it - sorry if it was already discussed and rejected or implicitly discussed and not grasped by me :)

erickmartins commented 1 year ago

This should be fixed on the next release - it's already on my personal dev branch and working as intended!

erickmartins commented 1 year ago

0.3.3 should fix both issues here.

pwalczysko commented 1 year ago

0.3.3 should fix both issues here.

Re: Preserving of color: the stroke color seems preserved, but the fill color is ignored.

Original: Screenshot 2023-01-26 at 19 57 55 Screenshot 2023-01-26 at 19 57 26

After transfer: Screenshot 2023-01-26 at 19 58 25 Screenshot 2023-01-26 at 19 59 47

pwalczysko commented 1 year ago

Re: Plate with an image with ROIs in original file: This works as intended. The ROIs are not doubled, and the masks are ignored. It seems that there is a nice output for the masks now

...
populating well 8269
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
not a supported ROI type
populating well 8268
populating well 8267
populating well 8266
pwalczysko commented 1 year ago

To be continued...

erickmartins commented 1 year ago

is there anything notable about the fillColors on the source and destination (different alpha values, etc)? is this an ROI with multiple shapes?

pwalczysko commented 1 year ago

is there anything notable about the fillColors on the source and destination (different alpha values, etc)?

Not on the first look, except for what you see on the screenshot - the fill on the destination image is white, the one on the source is purpleish.

is this an ROI with multiple shapes?

No, single shape.

@erickmartins Sorry, I think that in fact cli-transfer is doing the right thing. I have opened the transferred images in the Old inbuilt OMERO.web viewer, and the fills were transferred as expected. It is just the OMERO.iviewer which gives following discrepancy:

rgb(160, 90, 160) #this should be as it is like that on the original image (even in iviewer) rgba(160, 90, 160, 0) #this is what you can see on the transferred image, kind of "forced" by iviewer, indeed, when I try to draw a new ROI on an RGB image in iviewer, using the color picker UI only (ie. not typing into the text value box from which I copied the values here), then it insists on the rgba(x, y, z, 0) type of motive - this motive then results in empty fill, which is shown as white in the color picker box Screenshot 2023-01-27 at 16 45 16 Any ideas @will-moore ?

erickmartins commented 1 year ago

this is something I am unclear on from the OME model side (and maybe @will-moore can clarify): using fillColor.as_rgb_tuple() can return a tuple with 3 or 4 values, with the fourth value being alpha/transparency. If there are only 3 values, should I assume the fourth to be 0? 1? 255? I think for strokeColor I assume 1 for fully opaque; is fillColor the same?

will-moore commented 1 year ago

I don't know about as_rgb_tuple() but I would assume if there's only 3 values then alpha is 1.

erickmartins commented 1 year ago

alright, I'll make sure fillColor uses that as well

erickmartins commented 1 year ago

as of #42 fillColor assumes alpha to be 1 in case it's not there.