-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add a visualisation example of OMERO.tables #2354
Conversation
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/omero-tables-display-on-images-link-questions/87870/5 |
omero/developers/Tables.rst
Outdated
|
||
def create_table(connection, dataset_id): | ||
dataset = connection.getObject('Dataset', dataset_id) | ||
images = [connection.getObject('Image', child.getId()) for child in dataset.listChildren()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can just be images = dataset.listChildren()
. That gives you a generator of ImageWrappers.
or if you expect to iterate through the images multiple times, then convert the generator to a list:
images = list(dataset.listChildren())
Thanks for the contribution.
We have some OMERO.tables example python code at https://omero.readthedocs.io/en/v5.6.9/developers/Python.html#omero-tables and it would be good to avoid duplication (but also make sure that it's findable (I don't know if you've read those docs - linking to that from the OMERO.tables page might be a good idea). I think that example is missing the "Image" column. We could replace that table creation code with your example. Thanks! |
omero/developers/Tables.rst
Outdated
for image in images: | ||
data.append( | ||
{ | ||
'image': image.getId(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's kinda minor point but the convention we've adopted now is to use Image
rather than image
as the column name. E.g. see https://github.com/ome/omero-metadata#populate. I realise we've used image
in some of the older IDR tables (and it certainly works), but more recent ones (created by the current omero-metadata
tool) use Image
.
omero/developers/Tables.rst
Outdated
gateway.connect() | ||
gateway.keepAlive() | ||
gateway.SERVICE_OPTS.setOmeroGroup(group_id) | ||
gateway.setGroupForSession(group_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of requiring group_id
as an input, you can first lookup the Dataset since by default the gateway will do a cross-group query (or you can enforce that with gateway.SERVICE_OPTS.setOmeroGroup(-1)
).
Then you can get the group from the Dataset and use this, which you will need for saving table etc.
group_id = dataset.getDetails().group.id.val
conn.SERVICE_OPTS.setOmeroGroup(group_id)
omero/developers/Tables.rst
Outdated
dataset_id = "" # Dataset with some test images in it | ||
|
||
password = getpass(prompt='OMERO account password: ') | ||
gateway = BlitzGateway(user_name, password, host=host_name, secure=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By convention we use conn
for the BlitzGateway connection everywhere else in our docs etc.
If you want to create this from cli_login, you can do:
with cli_login() as cli:
conn = BlitzGateway(client_obj=cli._client)
In editing your example with the various points I was making above, I thought I'd push it so you can see: https://github.com/will-moore/python-scripts/blob/main/omero_tables_docs.py |
No, I had not tried the example your posted in the forum with the omero2pandas module. I stumbled across it before, but decided that I would like to do without that dependency - given that the module is recent. Your code review contains good lessons. Obviously it is neater without the little On the code page, I think an example that can be easily used on one's own OMERO.server instance would make things easier to get started. On this page, an explicit, visible mention of the image-column-name-convention would be really necessary in my opinion. Finally a small diagram and/or screenshot of the table display might help to understand how OMERO.tables are intended to function:
|
I understand the preference for writing your own code that you understand and have control over, instead of adding an unknown dependency. I suggest you:
Hardcoding the |
I will try it for my next script.
Sure, it was just a thought.
To sum up: For the client
...or could it be a Your suggestions resonate with me. One more comment concerning your third point
Instead of the initially proposed subsection, a new section (heading 2) named something like: Data visualisation (in OMERO.web) might make more sense. Do you want me to pack this in another pull request, or do you prefer to do the changes yourself? |
Sounds great - I opened a code PR at #2355, using a combination of your code sample and the one that was there already. |
Replaced by #2357 |
As a result of the discussions in the forum:
https://forum.image.sc/t/omero-tables-display-on-images-link-questions/87870