ome / omero-py

Python project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
20 stars 31 forks source link

added `_repr_html_` method for `_ImageWrapper` #394

Closed jo-mueller closed 2 months ago

jo-mueller commented 5 months ago

Fixes #393

Description

This PR overwrites the repr_html for the _imageWrapper class to show a html overview object in Jupyter Notebooks whenever an ImageWrapper object is sown in a cell. In essence, it creates an html object and fills it with some essential information about the image. Currently displayed:

Let me know what else you would consider relevant or added/changed in the displayed object. I am by no means an html expert, so there is certainly some room for improvement there :)

will-moore commented 5 months ago

This looks good:

Screenshot 2024-01-31 at 15 51 51

You're missing Dataset info. You can get a Dataset with self.getParent() although it might return None if more than one (or orphaned). Also getProject() could return None so you need to handle that possibility.

I had a go with this...


        def obj_html(obj, otype):
            return f"""<tr>
                    <td><b>{otype}</b></td>
                    <td class=text-right>{obj.id if obj else ""}</td>
                    <td class=text-right>{obj.name if obj else ""}</td>
                    </tr>
                """

        # create a sub-table for image information
        table_imageinfo = f"""
        <table>
            <tr><th></th><th>ID</th><th>Name</th></tr>
            {obj_html(self, 'Image')}
            {obj_html(self.getParent(), 'Dataset')}
            {obj_html(self.getProject(), 'Project')}
        </table>
        """

Screenshot 2024-01-31 at 16 17 05

will-moore commented 5 months ago

cc @pwalczysko @jburel - Any suggestions here?

pwalczysko commented 5 months ago

cc @pwalczysko @jburel - Any suggestions here?

What about pixel type and ROIs (number of ROIs) ?

jo-mueller commented 5 months ago

@will-moore I think it'll make sense to reduce the precision on the displayed voxel dimensions. Should be an easy fix. I do like the idea about adding some basic information about the parent dataset

Also getProject() could return None so you need to handle that possibility.

I think str(None) just returns "None". It wouldn't exactly be pretty but I think a None entry in any of these values would at least not break the displayed html

I had a go with this...

Would you be interested in sending this as a PR to this PR? Also, just that I understand it correctly - the obj_html function just makes the formulation of the actual html element a bit more readable, right?

What about pixel type and ROIs (number of ROIs) ?

I thought about ROIs but eventually refrained from adding it to restrict myself a bit to the parameters that would always be set for an image. But then again, there is still plenty of space in the table(s) 🤔

Edit: 1e40b2acb4827a014af3cb1bb7311a742482963e should limit floating point precision to three decimals. Do you think that's enough?

will-moore commented 5 months ago

Opened PR with those changes at https://github.com/jo-mueller/omero-py/pull/1

jo-mueller commented 5 months ago

Limited floating number precision makes it a bit prettier:

Capture

jo-mueller commented 5 months ago

Cool! Let me know if I should change anything - I probably won't have so much time to work on it because I'll be on leave for a few months soon 👍

jo-mueller commented 2 months ago

@will-moore @pwalczysko is there anything I should add to this PR to see it through? Happy for any feedback 👍

will-moore commented 2 months ago

Hi @jo-mueller - apologies for dropping this. I'd like to get a second opinion from someone on the OME team e.g. @jburel but in the meantime, it occurred to me that it might be nicer not to add all this html into the main gateway.__init__.py since it's already getting very verbose. Instead, the bulk of it could be moved to e.g. omero.gateway.utils as an image_to_html(image) method.

Also, in order to accept external contributions to any of the OME's GPL repos, we need to ask that you fill out a CLA form and submit it as described at https://ome-contributing.readthedocs.io/en/latest/cla.html

Thanks

jo-mueller commented 2 months ago

@will-moore thans for the reply - I sent the CLA form to the given address.

Is the ImageWrapper derived from the BlitzObjectWrapper? E.g., is this place where you would put it?

will-moore commented 2 months ago

Thanks for the CLA.

I would add the code to https://github.com/ome/omero-py/blob/master/src/omero/gateway/utils.py e.g.

import base64

...

def image_repr_html(image):
    ...

   table_imageinfo = f"""
        <table>
            <tr><th></th><th>ID</th><th>Name</th></tr>
            {obj_html(image, 'Image')}
            {obj_html(image.getParent(), 'Dataset')}
            {obj_html(image.getProject(), 'Project')}
        </table>
        """
    ...

    return html

Then add image_repr_html to the imports at https://github.com/ome/omero-py/blob/master/src/omero/gateway/__init__.py#L32

Then...

def _repr_html_(self):

    return image_repr_html(self)
jo-mueller commented 2 months ago

Done in da9d6443549e8353ba3cd8c5aa4c952dca95620e

will-moore commented 2 months ago

Final test with this branch installed locally, looking good...

Screenshot 2024-04-16 at 12 24 33

pwalczysko commented 2 months ago

With an image which does have pixelsizes, I have success

Screenshot 2024-04-16 at 12 49 03

But with an image which does not have pixelsizes, I do get an error (image created by touch a.fake cmd):

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
File ~/opt/anaconda3/envs/jotest/lib/python3.9/site-packages/IPython/core/formatters.py:344, in BaseFormatter.__call__(self, obj)
    342     method = get_real_method(obj, self.print_method)
    343     if method is not None:
--> 344         return method()
    345     return None
    346 else:

File ~/Work/omero-py/target/omero/gateway/__init__.py:299, in BlitzObjectWrapper._repr_html_(self)
    294 def _repr_html_(self):
    295     """
    296     Returns an HTML representation of the object. This is used by the
    297     IPython notebook to display the object in a cell.
    298     """
--> 299     return image_to_html(self)

File ~/Work/omero-py/target/omero/gateway/utils.py:276, in image_to_html(image)
    269 encoded_image = base64.b64encode(image.getThumbnail()).decode('utf-8')
    270 dimensions = f"""(
    271     {image.getSizeT()},
    272     {image.getSizeC()},
    273     {image.getSizeZ()},
    274     {image.getSizeY()},
    275     {image.getSizeX()})"""
--> 276 physical_dims = """({:.3f}, {:.3f}, {:.3f})""".format(
    277     image.getPixelSizeZ(),
    278     image.getPixelSizeY(),
    279     image.getPixelSizeX())
    280 physical_units = f"""(
    281     {image.getPixelSizeZ(units=True).getUnit()},
    282     {image.getPixelSizeY(units=True).getUnit()},
    283     {image.getPixelSizeX(units=True).getUnit()})"""
    285 table_dimensions = f"""
    286 <table>\n
    287     <tr>\n
   (...)
    299 </table>
    300 """

TypeError: unsupported format string passed to NoneType.__format__
jo-mueller commented 2 months ago

Hi @pwalczysko ,

thanks for the review, good catch!

I added some code to escape this error if the properties haven't been set (b0a7fada46d8af0764eeb94882474a610fbadde4). In this case, I would now get this - also shows that this is not a problem for orphaned images, i.e. if Dataset and Project are unspecified.

capture

pwalczysko commented 2 months ago

Confirming the fix

Screenshot 2024-04-19 at 14 09 37

@jo-mueller Thank you