Roi and plate fixes (#40)

* passing colors/width correctly

* removing debug print

* added provenance metadata for plates as well (#12)

* updating metadata test

* readme update, default `font_size` for `Label`s

* delete all file ROIs and change default strokecolor, fontsize

* readme inline fixes

README.md

@@ -34,7 +34,7 @@ Creates a transfer packet for moving objects between OMERO server instances.
 
 The syntax for specifying objects is: `<object>:<id>` where `<object>` can be `Image`, `Project`, `Dataset`, `Plate` or `Screen`. `Project` is assumed if `<object>:` is omitted. A file path needs to be provided; a tar file with the contents of the packet will be created at the specified path.
 
-Currently, only MapAnnotations, Tags, FileAnnotations and CommentAnnotations are packaged into the transfer pack. All kinds of ROI should work.
+Currently, only MapAnnotations, Tags, FileAnnotations and CommentAnnotations are packaged into the transfer pack. All kinds of ROI (except Masks) should work.
 
 Note that, if you are packing a `Plate` or `Screen`, default OMERO settings prevent you from downloading Plates and you will generate an empty pack file if you do so. If you want to generate a pack file from these entities, you will need to set `omero.policy.binary_access` appropriately.
 
@@ -54,6 +54,8 @@ omero transfer pack 999 tarfile.tar  # equivalent to Project:999
 
 Unpacks an existing transfer packet, imports images/plates as orphans and uses the XML contained in the transfer packet to re-create links, annotations and ROIs.
 
+Note that unpack needs to be able to identify the images it imports inequivocally; this can be a problem in case you have other images with the same `clientPath` (i.e. that were imported from the exact same location, including filename) and no annotations created by omero-cli-transfer. The most common case to generate this issue is an unpack that fails after the import step - the lingering images are not annotated correctly and a retry of the same unpack will use the same `clientPath` and cause issues. The best solution is cleaning up after failed unpacks.
+
 `--ln_s` forces imports to use the transfer=ln_s option, in-place importing files. Same restrictions of regular in-place imports apply.
 
 `--output` allows for specifying an optional output folder where the packet will be unzipped.

src/generate_omero_objects.py

@@ -77,6 +77,8 @@ def create_annotations(ans: List[Annotation], conn: BlitzGateway, hash: str,
                         key_value_data.append(['zip_file_md5', hash])
                     if v.k == "origin_image_id" and "img_id" in metadata:
                         key_value_data.append([v.k, v.value])
+                    if v.k == "origin_plate_id" and "plate_id" in metadata:
+                        key_value_data.append([v.k, v.value])
                     if v.k == "packing_timestamp" and "timestamp" in metadata:
                         key_value_data.append([v.k, v.value])
                     if v.k == "software" and "software" in metadata:
@@ -167,10 +169,24 @@ def create_plate_map(ome: OME, img_map: dict, conn: BlitzGateway
             params,
             conn.SERVICE_OPTS
             )
-
-        if results:
+        all_plate_ids = list(set(sorted([r[0].val for r in results])))
+        plate_ids = []
+        for pl_id in all_plate_ids:
+            anns = ezomero.get_map_annotation_ids(conn, "Plate", pl_id)
+            if not anns:
+                plate_ids.append(pl_id)
+            else:
+                is_annotated = False
+                for ann in anns:
+                    ann_content = conn.getObject("MapAnnotation", ann)
+                    if ann_content.getNs() == \
+                            'openmicroscopy.org/cli/transfer':
+                        is_annotated = True
+                if not is_annotated:
+                    plate_ids.append(pl_id)
+        if plate_ids:
             # plate was imported as plate
-            plate_id = results[0][0].val
+            plate_id = plate_ids[0]
         else:
             # plate was imported as images
             plate_id = create_plate_from_images(plate, img_map, conn)
@@ -302,21 +318,27 @@ def create_rois(rois: List[ROI], imgs: List[Image], img_map: dict,
                 if len(fc) == 3:
                     fill_color = fc + (0,)
                 else:
-                    fill_color = fc
+                    alpha = fc[3] * 255
+                    fill_color = fc[0:3] + (int(alpha),)
             else:
                 fill_color = (0, 0, 0, 0)
             if roi.union[0].stroke_color:
                 sc = roi.union[0].stroke_color.as_rgb_tuple()
                 if len(sc) == 3:
-                    stroke_color = sc + (0,)
+                    stroke_color = sc + (255,)
                 else:
                     stroke_color = sc
             else:
-                stroke_color = (0, 0, 0, 0)
+                stroke_color = (255, 255, 255, 255)
+            if roi.union[0].stroke_width:
+                stroke_width = int(roi.union[0].stroke_width)
+            else:
+                stroke_width = 1
             img_id_dest = img_map[img.id]
             ezomero.post_roi(conn, img_id_dest, shapes, name=roi.name,
                              description=roi.description,
-                             fill_color=fill_color, stroke_color=stroke_color)
+                             fill_color=fill_color, stroke_color=stroke_color,
+                             stroke_width=stroke_width)
     return
 
 

src/generate_xml.py

@@ -134,6 +134,8 @@ def create_point(shape: PointI) -> Point:
         args['locked'] = shape.getLocked().val
     if shape.getStrokeColor() is not None:
         args['stroke_color'] = shape.getStrokeColor().val
+    if shape.getStrokeWidth() is not None:
+        args['stroke_width'] = shape.getStrokeWidth().getValue()
     pt = Point(**args)
     return pt
 
@@ -160,6 +162,8 @@ def create_line(shape: LineI) -> Line:
         args['locked'] = shape.getLocked().val
     if shape.getStrokeColor() is not None:
         args['stroke_color'] = shape.getStrokeColor().val
+    if shape.getStrokeWidth() is not None:
+        args['stroke_width'] = shape.getStrokeWidth().getValue()
     if shape.getMarkerStart() is not None:
         args['marker_start'] = shape.getMarkerStart().val
     if shape.getMarkerEnd() is not None:
@@ -190,6 +194,8 @@ def create_rectangle(shape: RectangleI) -> Rectangle:
         args['locked'] = shape.getLocked().val
     if shape.getStrokeColor() is not None:
         args['stroke_color'] = shape.getStrokeColor().val
+    if shape.getStrokeWidth() is not None:
+        args['stroke_width'] = shape.getStrokeWidth().getValue()
     rec = Rectangle(**args)
     return rec
 
@@ -216,6 +222,8 @@ def create_ellipse(shape: EllipseI) -> Ellipse:
         args['locked'] = shape.getLocked().val
     if shape.getStrokeColor() is not None:
         args['stroke_color'] = shape.getStrokeColor().val
+    if shape.getStrokeWidth() is not None:
+        args['stroke_width'] = shape.getStrokeWidth().getValue()
     ell = Ellipse(**args)
     return ell
 
@@ -240,6 +248,8 @@ def create_polygon(shape: PolygonI) -> Polygon:
         args['locked'] = shape.getLocked().val
     if shape.getStrokeColor() is not None:
         args['stroke_color'] = shape.getStrokeColor().val
+    if shape.getStrokeWidth() is not None:
+        args['stroke_width'] = shape.getStrokeWidth().getValue()
     pol = Polygon(**args)
     return pol
 
@@ -264,6 +274,8 @@ def create_polyline(shape: PolylineI) -> Polyline:
         args['locked'] = shape.getLocked().val
     if shape.getStrokeColor() is not None:
         args['stroke_color'] = shape.getStrokeColor().val
+    if shape.getStrokeWidth() is not None:
+        args['stroke_width'] = shape.getStrokeWidth().getValue()
     pol = Polyline(**args)
     return pol
 
@@ -272,10 +284,12 @@ def create_label(shape: LabelI) -> Label:
     args = {'id': shape.getId().val, 'x': shape.getX().val,
             'y': shape.getY().val}
     args['text'] = shape.getTextValue().val
-    args['font_size'] = shape.getFontSize().getValue()
+    args['font_size'] = 10
     args['the_c'] = 0
     args['the_z'] = 0
     args['the_t'] = 0
+    if shape.getFontSize() is not None:
+        args['font_size'] = shape.getFontSize().getValue()
     if shape.getTheC() is not None:
         args['the_c'] = max(shape.getTheC().val, 0)
     if shape.getTheZ() is not None:
@@ -288,6 +302,8 @@ def create_label(shape: LabelI) -> Label:
         args['locked'] = shape.getLocked().val
     if shape.getStrokeColor() is not None:
         args['stroke_color'] = shape.getStrokeColor().val
+    if shape.getStrokeWidth() is not None:
+        args['stroke_width'] = shape.getStrokeWidth().getValue()
     pt = Label(**args)
     return pt
 
@@ -317,7 +333,7 @@ def create_shapes(roi: RoiI) -> List[Shape]:
             lab = create_label(s)
             shapes.append(lab)
         else:
-            print("not a real thing")
+            print("not a supported ROI type")
             continue
     return shapes
 
@@ -389,7 +405,7 @@ def create_filepath_annotations(id: str, conn: BlitzGateway,
 
 def create_provenance_metadata(conn: BlitzGateway, img_id: int,
                                hostname: str,
-                               metadata: Union[List[str], None]
+                               metadata: Union[List[str], None], plate: bool
                                ) -> Union[Tuple[MapAnnotation, AnnotationRef],
                                           Tuple[None, None]]:
     if not metadata:
@@ -404,8 +420,12 @@ def create_provenance_metadata(conn: BlitzGateway, img_id: int,
     db_id = conn.getConfigService().getDatabaseUuid()
 
     md_dict: Dict[str, Any] = {}
-    if "img_id" in metadata:
-        md_dict['origin_image_id'] = img_id
+    if plate:
+        if "plate_id" in metadata:
+            md_dict['origin_plate_id'] = img_id
+    else:
+        if "img_id" in metadata:
+            md_dict['origin_image_id'] = img_id
     if "timestamp" in metadata:
         md_dict['packing_timestamp'] = date_time
     if "software" in metadata:
@@ -471,7 +491,7 @@ def populate_image(obj: ImageI, ome: OME, conn: BlitzGateway, hostname: str,
                                         description=desc, pixels=pix)
     for ann in obj.listAnnotations():
         add_annotation(img, ann, ome, conn)
-    kv, ref = create_provenance_metadata(conn, id, hostname, metadata)
+    kv, ref = create_provenance_metadata(conn, id, hostname, metadata, False)
     if kv:
         kv_id = f"Annotation:{str(kv.id)}"
         if kv_id not in [i.id for i in ome.structured_annotations]:
@@ -561,6 +581,13 @@ def populate_plate(obj: PlateI, ome: OME, conn: BlitzGateway,
     pl, pl_ref = create_plate_and_ref(id=id, name=name, description=desc)
     for ann in obj.listAnnotations():
         add_annotation(pl, ann, ome, conn)
+    kv, ref = create_provenance_metadata(conn, id, hostname, metadata, True)
+    if kv:
+        kv_id = f"Annotation:{str(kv.id)}"
+        if kv_id not in [i.id for i in ome.structured_annotations]:
+            ome.structured_annotations.append(kv)
+        if ref:
+            pl.annotation_ref.append(ref)
     for well in obj.listChildren():
         well_obj = conn.getObject('Well', well.getId())
         well_ref = populate_well(well_obj, ome, conn, hostname, metadata)

src/omero_cli_transfer.py

@@ -178,7 +178,7 @@ def _configure(self, parser):
         )
         unpack.add_argument(
             "--metadata",
-            choices=['all', 'none', 'img_id', 'timestamp',
+            choices=['all', 'none', 'img_id', 'plate_id', 'timestamp',
                      'software', 'version', 'md5', 'hostname', 'db_id',
                      'orig_user', 'orig_group'], nargs='+',
             help="Metadata field to be added to MapAnnotation"
@@ -261,7 +261,7 @@ def _process_metadata(self, metadata: Union[List[str], None]):
             metadata = ['all']
         if "all" in metadata:
             metadata.remove("all")
-            metadata.extend(["img_id", "timestamp", "software",
+            metadata.extend(["img_id", "plate_id", "timestamp", "software",
                              "version", "hostname", "md5", "orig_user",
                              "orig_group"])
         if "none" in metadata:
@@ -339,6 +339,7 @@ def __unpack(self, args):
             ln_s = False
         dest_img_map = self._import_files(folder, filelist,
                                           ln_s, args.skip, self.gateway)
+        self._delete_all_rois(dest_img_map, self.gateway)
         print("Matching source and destination images...")
         img_map = self._make_image_map(src_img_map, dest_img_map)
         print("Creating and linking OMERO objects...")
@@ -427,6 +428,15 @@ def _import_files(self, folder: Path, filelist: List[str], ln_s: bool,
             dest_map[dest_path] = img_ids
         return dest_map
 
+    def _delete_all_rois(self, dest_map: dict, gateway: BlitzGateway):
+        roi_service = gateway.getRoiService()
+        for imgs in dest_map.values():
+            for img in imgs:
+                result = roi_service.findByImage(img, None)
+                for roi in result.rois:
+                    gateway.deleteObject(roi)
+        return
+
     def _get_image_ids(self, file_path: str, conn: BlitzGateway) -> List[str]:
         """Get the Ids of imported images.
         Note that this will not find images if they have not been imported.

test/unit/test_set.py

@@ -48,14 +48,14 @@ def test_process_metadata(self):
         metadata = None
         self.transfer._process_metadata(metadata)
         assert set(self.transfer.metadata) == \
-            set(["img_id", "timestamp", "software", "version", "hostname",
-                 "md5", "orig_user", "orig_group"])
+            set(["img_id", "plate_id", "timestamp", "software", "version",
+                 "hostname", "md5", "orig_user", "orig_group"])
         self.transfer.metadata = []
         metadata = ['all', 'db_id']
         self.transfer._process_metadata(metadata)
         assert set(self.transfer.metadata) == \
-            set(["img_id", "timestamp", "software", "version", "hostname",
-                 "md5", "orig_user", "orig_group", "db_id"])
+            set(["img_id", "plate_id", "timestamp", "software", "version",
+                 "hostname", "md5", "orig_user", "orig_group", "db_id"])
         self.transfer.metadata = []
         metadata = ['none', 'db_id']
         self.transfer._process_metadata(metadata)