Roi name allowed none - Githubissues

will-moore commented 3 years ago

In the case when we populate metadata on a target Image with a csv as in the README (below), the ROI IDs are already in the csv and we don't need the ROI.name in OMERO to identify which row applies to which ROI. However, without this PR, any ROIs without the name set in OMERO would cause populate metadata to fail.

    # header roi,l,d,l
    Roi,Shape,probability,area
    501,1,0.8,250
    502,1,0.9,500
    503,1,0.2,25

To test:

Draw some Shapes on an Image in iviewer and Save
Note the ROI IDs from iviewer and update the ROIs in a csv like the one above.
Run populate metadata script on the Image in webclient with the csv (this requires https://github.com/ome/omero-scripts/pull/184) or use the CLI.
Should get an OMERO.table created on the Image with a Roi Name column where all the values will be empty
Bonus: If you also include a Shape column of type l with the correct Shape values, it should be possible to use this OMERO.table to create a heatmap on those Shapes in OMERO.figure as described in https://github.com/ome/omero-guide-figure/pull/11

cc @pwalczysko

will-moore commented 3 years ago

Seems ALL tables tests failed with:

    def newTable(self, repoId, path, _ctx=None):
>       return _M_omero.grid.SharedResources._op_newTable.invoke(self, ((repoId, path), _ctx))
E       omero.InternalException: exception ::omero::InternalException
E       {
E           serverStackTrace = 
E           serverExceptionClass = 
E           message = null table as argument
E       }

as if pytables install failed or something?

joshmoore commented 3 years ago

It could have either fallen over today (happens very rarely) or a code bug in an open PR prevented it from starting.

pwalczysko commented 3 years ago

On merge-ci on image https://merge-ci.openmicroscopy.org/web/webclient/?show=image-144515 I am getting. an error as below (the csv I used is attached to that image)

WARNING:omero_metadata.populate:Conflicting ROI names.
WARNING:omero_metadata.populate:Conflicting ROI names.
Traceback (most recent call last):
  File "./script", line 125, in populate_metadata
    ctx.parse_from_handle(data)
AttributeError: 'ParsingContext' object has no attribute 'parse_from_handle'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./script", line 182, in <module>
    run_script()
  File "./script", line 174, in run_script
    message = populate_metadata(client, conn, script_params)
  File "./script", line 129, in populate_metadata
    ctx.preprocess_from_handle(data_for_preprocessing)
  File "/home/omero/workspace/OMERO-server/.venv3/lib64/python3.6/site-packages/omero_metadata/populate.py", line 1055, in preprocess_from_handle
    self.column_types = HeaderResolver.get_column_types(first_row)
  File "/home/omero/workspace/OMERO-server/.venv3/lib64/python3.6/site-packages/omero_metadata/populate.py", line 201, in get_column_types
    column_types = parse_column_types(column_types)
  File "/home/omero/workspace/OMERO-server/.venv3/lib64/python3.6/site-packages/omero_metadata/populate.py", line 2138, in parse_column_types
    raise MetadataError(message)
omero_metadata.populate.MetadataError: 
Column type '    roi' unknown.
Choose from following: plate,well,image,dataset,roi,d,l,s,b
Exception ignored in: <bound method _TemporaryFileCloser.__del__ of <tempfile._TemporaryFileCloser object at 0x7fadffb1c208>>
Traceback (most recent call last):
  File "/usr/lib64/python3.6/tempfile.py", line 452, in __del__
    self.close()
  File "/usr/lib64/python3.6/tempfile.py", line 448, in close
    unlink(self.name)
FileNotFoundError: [Errno 2] No such file or directory: '/home/omero/omero/tmp/omero_omero/55477/populate_roi0n7hz8zcdir/tmpy6cpv3tk'
Exception ignored in: <bound method _TemporaryFileCloser.__del__ of <tempfile._TemporaryFileCloser object at 0x7fadffb1c5f8>>
Traceback (most recent call last):
  File "/usr/lib64/python3.6/tempfile.py", line 452, in __del__
    self.close()
  File "/usr/lib64/python3.6/tempfile.py", line 448, in close
    unlink(self.name)
FileNotFoundError: [Errno 2] No such file or directory: '/home/omero/omero/tmp/omero_omero/55477/populate_roi0n7hz8zcdir/tmpqeoo4b6s'

Does that suggest that this feature is not installed on merge-ci ? If not, how to test ?

will-moore commented 3 years ago

@pwalczysko The error I see there is:

Column type '    roi' unknown.
Choose from following: plate,well,image,dataset,roi,d,l,s,b

So it looks like you've got some extra whitespace in the header.

I just tried with image https://merge-ci.openmicroscopy.org/web/webclient/?show=image-83221 and the csv below:

# header roi,l,d,l
Roi,Shape,probability,area
273462,279241,0.8,250

using the webclient to pick the csv from the script dialog. You can see that table with 1 row was created: https://merge-ci.openmicroscopy.org/web/webclient/omero_table/649358/

NB: this also tests that https://github.com/ome/omero-scripts/pull/184 is working.

pwalczysko commented 3 years ago

thank you @will-moore , there was a tab preceding the first hash and the second line starting with "Roi". I removed the tab from the csv and it started to work. I can see that the table has nothing in the ROI name column as indicated. https://merge-ci.openmicroscopy.org/web/webclient/omero_table/649360/

This looks good to me.

pwalczysko commented 3 years ago

The "bonus" workflow fails for me though. The script in Console produces a link https://merge-ci.openmicroscopy.org/webgateway/table/Image/144515/query/?query=Shape-480591 which gives 404. But the ROI indeed has the correct shape as verified on https://merge-ci.openmicroscopy.org/web/iviewer/?roi=425062 . It seems to me that should work, but it does not...

will-moore commented 3 years ago

Ah, I see - Because merge-ci is deployed with the /web/ URL prefix, that needs be added to the URL in the script. to give: https://merge-ci.openmicroscopy.org/web/webgateway/table/Image/144515/query/?query=Shape-480591

I've updated the script in the PR to handle any change in the URL prefix at https://github.com/ome/omero-guide-figure/pull/11/commits/0d314b433153ca058cdd6e5bf67f2e974f810a43

With that change, and loading probability column, that gave me heatmap on shapes at: https://merge-ci.openmicroscopy.org/web/figure/file/76582

will-moore commented 3 years ago

OK - this is getting me confused because the "Bonus" was just to complete the figure workflow, but is not related to this PR, which is otherwise good to merge. So we should move the OMERO.figure scripting discussion to https://github.com/ome/omero-guide-figure/pull/11

pwalczysko commented 3 years ago

the funclionality indeed works as described, and the "bonus" test is actually working now too. Ready to merge fmpov.

sbesson commented 3 years ago

Overall the proposed change makes sense to me. In the case where ROIs are specified by ID, the requirement for a non-null name attribute seems unnecessary as the resolution should be unambiguous. @erindiel can you confirm relaxing this requirement in this scenario will not cause issues in your workflows?

erindiel commented 3 years ago

Thanks @will-moore and @sbesson . Agreed that the current requirement for a non-null ROI Name should not be the case when ROI ID is provided. Thanks for the change, and no issues for our workflows.

ome / omero-metadata

Roi name allowed none #58