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

Populate metadata Image dtype #184

Merged
merged 6 commits into from
Jun 25, 2021

Conversation

will-moore
Copy link
Member

This allows the Populate_Metadata script UI to pick Image types.

To test:

@will-moore will-moore changed the title Add 'Image' to OBJECT_TYPES Populate metadata Image dtype Feb 24, 2021
@sbesson
Copy link
Member

sbesson commented Feb 24, 2021

I am aware that ome/omero-cli-server#5 contains the ongoing discussion about how to express the dependencies for this Python packages on OMERO.server. However, I wonder whether setup.py should not include and install_requires block that would capture these requirements as well (in the case of this PR, I assume omero-metadata>=0.6.0)

@sbesson
Copy link
Member

sbesson commented Feb 25, 2021

Tested in conjunction with ome/omero-web#264. The script works as expected, taking the input CSV and generating a table alongside the original CSV

Screenshot 2021-02-25 at 16 08 34

Two additional thoughts:

  • should the script help be updated to reflect the fact that metadata can be populated for ROIs at the level of the images?
  • coming back to the considerations Populate metadata Image dtype #184 (comment), do we need to handle the case where the scripts would be upgraded by not the underlying library?

@will-moore
Copy link
Member Author

We only add Image to the types if from omero_metadata.populate import ParsingContext passes.
However, if I have installed an older version of omero-metadata e.g. v0.5.1 when Image wasn't supported, I get this error if I try to use an Image:

    self.value_resolver = ValueResolver(client, target_object)
  File "/Users/wmoore/Desktop/METADATA/omero-metadata/src/omero_metadata/populate.py", line 338, in __init__
    'Unsupported target object class: %s' % self.target_class)
omero_metadata.populate.MetadataError: Unsupported target object class: <class 'omero.model.ImageI'>

I guess we could detect the support for Image with from omero_metadata.populate import ImageWrapper, but that might not be very reliable. It could be renamed or removed.
I don't know if there's a reliable way to get the version number and show "Please upgrade" if it's not recent or latest?

Otherwise, we should at least state on the omero-metadata README (and omero-guides if they mention this workflow) that Image/ROIs support was added in v0.6.0. Although, a user won't necessarily know what version is installed on their server.

@will-moore
Copy link
Member Author

That last commit tests to see if ImageWrapper is supported in the current version of omero-metadata if the user has chosen an "Image" dtype.
If not, return a suitable message:

Screenshot 2021-06-16 at 12 32 04

Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

I like the feedback to the users if it's not installed. (Better would of course be if scripts could have dependencies. One day...)

I could see having the "Please update" message have a red X rather than a green check, but that's not a strong feeling. 👍

@joshmoore joshmoore merged commit b8fd012 into ome:develop Jun 25, 2021
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.

3 participants