ome / omero-figure

An OMERO.web app for creating Figures from images in OMERO
http://figure.openmicroscopy.org
GNU Affero General Public License v3.0
15 stars 31 forks source link

Rois from omero2 #190

Closed will-moore closed 7 years ago

will-moore commented 7 years ago

Previously opened as https://github.com/ome/omero-figure/pull/165 Ported to new code layout by squashing commits etc.

This allows loading of ROIs from OMERO to add to figure panels as new Shapes.

To test:

screen shot 2017-02-01 at 14 52 03

Follow up ideas:

waxenegger commented 7 years ago

Great stuff @will-moore. I like the drawing and rotation abilities.

That seems to all work as you described it, tried a rotated ellipse as well.

image

After changing stroke width and color, I wanted more and change the fill color. Maybe another follow-up idea...

will-moore commented 7 years ago

To test the upgrade of Shapes json format from 5.2.x format to 5.3.x (ellipse.cx -> ellipse.x etc) we need to:

Then I will Open this PR again...

bramalingam commented 7 years ago

Created figure without this PR on eel.merge, http://web-dev-merge.openmicroscopy.org/figure/file/17602 (user-1)

Lots of ellipses and I have a copy of the JSON file as well : yes it is "version":1:

{"version":1,"panels":[{"labels":[],"height":94.18951612903223,"channels":....

will-moore commented 7 years ago

After upgrade, we also need to test that figure export script works, as TIFF and PDF. (will need to manually upload the script).

jburel commented 7 years ago
will-moore commented 7 years ago

@jburel

jburel commented 7 years ago

Let's discuss

will-moore commented 7 years ago

After discussion with @jburel, TODOs:

bramalingam commented 7 years ago

Tried exporting, http://web-dev-merge.openmicroscopy.org/figure/file/17602

A figure file which was created before this PR was opened. https://github.com/ome/omero-figure/pull/190#issuecomment-281746124

And got the following exceptions when I tried to export as pdf,

No handlers could be found for logger "figure_to_pdf" /usr/lib64/python2.6/site-packages/reportlab/lib/utils.py:653: DeprecationWarning: tostring() is deprecated. Please call tobytes() instead. self._data = im.tostring() Traceback (most recent call last): File "./script", line 1665, in run_script() File "./script", line 1651, in run_script file_annotation = export_figure(conn, script_params) File "./script", line 1609, in export_figure return fig_export.build_figure() File "./script", line 690, in build_figure self.add_panels_to_page(panels_json, image_ids, page) File "./script", line 1311, in add_panels_to_page self.add_rois(panel, page) # This does nothing for TIFF export File "./script", line 823, in add_rois self.page_height) File "./script", line 130, in init self.draw_ellipse(shape) File "./script", line 319, in draw_ellipse c = self.panel_to_page_coords(shape['x'], shape['y']) KeyError: 'x'

And for conversion to tiff,

No handlers could be found for logger "figure_to_pdf" Traceback (most recent call last): File "./script", line 1665, in run_script() File "./script", line 1651, in run_script file_annotation = export_figure(conn, script_params) File "./script", line 1609, in export_figure return fig_export.build_figure() File "./script", line 690, in build_figure self.add_panels_to_page(panels_json, image_ids, page) File "./script", line 1307, in add_panels_to_page image, pil_img = self.draw_panel(panel, page, i) File "./script", line 1140, in draw_panel self.paste_image(pil_img, img_name, panel, page, dpi) File "./script", line 1503, in paste_image ShapeToPilExport(pil_img, panel, crop) File "./script", line 381, in init self.draw_ellipse(shape) File "./script", line 518, in draw_ellipse ctr = self.get_panel_coords(shape['x'], shape['y']) KeyError: 'x'

bramalingam commented 7 years ago

Same errors, looks like an issue with getting the panel coordinates! Is this related to/caused by my width and height PR (we handled the page-wise x,y coordinate management in that PR)? https://github.com/ome/omero-figure/pull/169

jburel commented 7 years ago

see also https://trello.com/c/zGuNePnu/4-load-rois-from-omero

will-moore commented 7 years ago

@bramalingam I'm trying to understand the bug you found. The Ellipses that are giving you errors on the figure seem to come from OMERO (ID matches an Ellipse on that image in OMERO). Can you remember how you created that figure, since if the figure was created without this PR merged, you shouldn't have been able to load ROIs from OMERO. Maybe that came from a previously created figure?

{"rotation":60.65125478487214,"strokeWidth":1,"type":"Ellipse","strokeColor":"#c3c4c4","id":1751}

I just tried this:

Does this sound feasible?

will-moore commented 7 years ago

To try reproduce the upgrade issue above, closing this PR so we can create figures without it merged.

will-moore commented 7 years ago

Recent changes to test:

waxenegger commented 7 years ago

Checked the most recent changes and they all work.

figure_rois

will-moore commented 7 years ago

@bramalingam Can you try testing the upgrade of shapes again? @jburel Do you want to check that workflow is behaving as we discussed above?

bramalingam commented 7 years ago

Rechecked the following scenario,

1) Created Figure with ROI's without this PR. ( json should contain "version 1") 2) fetched this branch and opened the same Figure, and exported as pdf.

Can no longer see the exception seen in : https://github.com/ome/omero-figure/pull/190#issuecomment-284007902

and can see version 2 in the json as well.

jburel commented 7 years ago

Looking at http://web-dev-merge.openmicroscopy.org/figure/file/17602/

jburel commented 7 years ago

RFE: A "undo" the copy paste for example will be nice (not in this PR)

jburel commented 7 years ago

Open the "ROI dialog", the tooltip of the Load ROIs button always indicates the number of rois of the first image I opened I will remove the number from the tooltip.

will-moore commented 7 years ago

@jburel - I can't reproduce the tooltip bug https://github.com/ome/omero-figure/pull/190#issuecomment-289783627

will-moore commented 7 years ago

As discussed with @bramalingam, when you "OK" the ROI dialog, the Z and T position of the dialog get saved to your figure. If you don't want to change the Z/T, then you can 'revert' to the original Z/T using new buttons:

screen shot 2017-03-29 at 12 31 45

bramalingam commented 7 years ago

The above functionality works as expected.

2 separate points to consider: 1) Display of the ROI/Shape ids from OMERO, and 2) Size/width of the rows/columns in the ROI table.

Also, an edge case workflow where an ellipse ROI/shape has its centre outside the bounds of the viewport, throws a not so informative dialog box. Maybe update the text in the dialog box or handle ellipse's better when there are near the edges of the image? (Can be ignored as well, because in a user workflow I wouldn't expect the ROI's around the edges to be useful in many ways)

will-moore commented 7 years ago

Added IDs to the ROI/Shapes table to look more like iViewer cc @waxenegger

screen shot 2017-03-31 at 13 44 05

waxenegger commented 7 years ago

Latest commits work as as intended. Shape Id is showing and tooltip gives me coordinates.

figure_load_rois

jburel commented 7 years ago

similar to iviewer: I am not sure about the shape/roi ID. Especially with the high number

jburel commented 7 years ago

Few comments:

jburel commented 7 years ago

@will-moore I cannot reproduce the roi count issue, I don't think it was ever a problem