Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added _repr_html_ method for _ImageWrapper #394

Merged
merged 6 commits into from
Apr 24, 2024

Conversation

jo-mueller
Copy link
Contributor

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:

  • Image ID
  • Project ID
  • Description
  • Image Name
  • Image dimensions (TCZYX)
  • Voxel dimensions (currently output with long names (e.g. MICROMETER - there is probably a way to add the units as µm or whatever it is set to).
  • Channel names

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 :)

@jo-mueller jo-mueller changed the title added repr_html method for _ImageWrapper added _repr_html_ method for _ImageWrapper Jan 30, 2024
@will-moore
Copy link
Member

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
Copy link
Member

cc @pwalczysko @jburel - Any suggestions here?

@pwalczysko
Copy link
Member

cc @pwalczysko @jburel - Any suggestions here?

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

@jo-mueller
Copy link
Contributor Author

jo-mueller commented Jan 31, 2024

@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: 1e40b2a should limit floating point precision to three decimals. Do you think that's enough?

@will-moore
Copy link
Member

Opened PR with those changes at jo-mueller#1

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Show Image, Dataset and Project in a table
@jo-mueller
Copy link
Contributor Author

Limited floating number precision makes it a bit prettier:

Capture

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks

@jo-mueller
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@will-moore
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link
Member

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
Copy link
Contributor Author

Done in da9d644

@will-moore
Copy link
Member

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

Screenshot 2024-04-16 at 12 24 33

@pwalczysko
Copy link
Member

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__

Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the image with no pixelsizes problem ^^^ thank you.

@jo-mueller
Copy link
Contributor Author

jo-mueller commented Apr 18, 2024

Hi @pwalczysko ,

thanks for the review, good catch!

I added some code to escape this error if the properties haven't been set (b0a7fad). 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
Copy link
Member

Confirming the fix

Screenshot 2024-04-19 at 14 09 37

@jo-mueller Thank you

@jburel jburel merged commit 65bdd13 into ome:master Apr 24, 2024
1 check passed
@jo-mueller jo-mueller deleted the add-`repr_html` branch August 5, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overwrite _repr_html_ for _ImageWrapper
4 participants