ome / omero-metadata

OMERO plugin for metadata manipulation https://www.openmicroscopy.org/omero/
GNU General Public License v2.0
7 stars 13 forks source link

Support images_by_name for Screen and Plate #64

Open will-moore opened 2 years ago

will-moore commented 2 years ago

See #63.

Supports parsing Image Name to Image ID for HCS data. Image Name must be unique to a Plate (similar to Dataset).

NB: There has been a long-standing inconsistency in the way that populate metadata handles column names for PDI vv HCS. For PDI, you start with "Image Name" column (with s type in header) in your csv. A new "Image" column is added with Image IDs. This seems to make the most sense. For HCS, you start with "Well" column (with well type in header), that contains the Well Names. This column is converted to a Well ID column (still named Well) and a new Well Name column is added, containing the names that were in the Well column. This is confusing!

In this case, so as not to have different handling of Image Name columns for HCS vv PDI, we need an Image Name column (type: s) alongside a Well column (type: well), even though they both contain names!

E.g. screen.csv

# header plate,well,s,s
Plate,Well,Image Name,Drug_name
plate1,A1,well 1 field 1, DMSO
plate1,A1,well 1 field 2, DMSO
plate1,A2,well 2 field 1, Drug1
plate1,A2,well 2 field 2, Drug1

E.g. plate.csv

# header well,s,s
Well,Image Name,Drug_name
A1,well 1 field 1, DMSO
A1,well 1 field 2, DMSO
A2,well 2 field 1, Drug1
A2,well 2 field 2, Drug1

TODO: add tests...

snoopycrimecop commented 2 years ago

Conflicting PR. Removed from build OMERO-plugins-push#988. See the console output for more details. Possible conflicts:

--conflicts

will-moore commented 2 years ago

I think I need to get #62 merged ahead of this PR... Will look at that now...

snoopycrimecop commented 2 years ago

Conflicting PR. Removed from build OMERO-plugins-push#1005. See the console output for more details. Possible conflicts:

--conflicts Conflict resolved in build OMERO-plugins-push#1383. See the console output for more details.

will-moore commented 2 years ago

Closing for now to avoid more conflicts...

JensWendt commented 1 year ago

Will this be re-opened at some point? Still hoping for the possibility to add ROI specific measurements to multiple images (aka positions) in one well.

will-moore commented 1 year ago

Re-opened and fixed merge conflicts. Hopefully tests will pass (not run locally yet).

@JensWendt it would be a great help if you can test this (see description) and let us know if it's working for you? Thanks!

JensWendt commented 1 year ago

Heyo @will-moore, thank you!! Will test it 👍 Which description? The one in the first post?

will-moore commented 1 year ago

Yes, with the screen.csv and plate.csv examples. Thx

JensWendt commented 1 year ago

Heyo @will-moore ,

I did some testing. With the CLI omero metadata populate ... and as well with the server side script Populate Metadata. I simply exchanged the current populate.py in /opt/omero/venv_server/lib/python3.6/site-packages/omero_metadata with your changed one from this branch. I think this was the correct way to test it!? Everything works as expected with one exception. screen.csv will produce the expected omero-table but will throw the warning WARNING:omero_metadata.populate:PlateColumn is unimplemented. The result looks like this, without the Plate column hyperlink functionality. grafik

From my perspective this is not a big problem as long as the table is correct. Good job! Thank you!!

JensWendt commented 1 year ago

I was playing around with ROI specific metadata for whole projects/screens. It works but - again - gives me the warning WARNING:omero_metadata.populate:PlateColumn is unimplemented. for a .csv with a Plate column containing plate names. The bulk annotations looks like this: grafik

No hyperlink functionality for either Plate or ROI. Again, no problem for me, but I thought it might be interesting to report.

If the pull request goes through maybe also adapt the readme.rst line "If the target is an Image or a Dataset, a CSV with ROI-level..." Because now it seems to work on every level :+1:

abhamacher commented 1 year ago

Hi everyone,

I also did some testing this morning with the following combinations of source csv data containing either:

Except of the warning message, that Jens already mentioned, I would like to point out the following concerns:

  1. When populating image-based data an "Image Number" is added to the OMERO.table. But this number is not the Image ID, which I find a bit confusing.

  2. When populating image-based data, OMERO.parade can properly load individual image data for example in the filter menu (see Example 68...366 per image) but the dots in the parade plot do not cover all images, but rather wells only, which is inconsistent.

(as a side note, with the same OMERO.table plots in parade-crossfilter show all individual image dots)

  1. Based on concern no. 3, using parade one cannot see if the data shown is coming from wells or from images. Typically someone would populate the OMERO.table but another or multiple persons would browse through the data, which are not aware of the underlying data structure. So this is something that should be visible somehow to the user.

  2. When populating roi-based data, the activity log returns an irritating message ("Failed to get results"), although the OMERO.table seems to be generated properly. populate1

  3. Issue no. 2-no. 4 also apply to roi-based data.

  4. I'm not a big fan of the order of columns. For example, placing well id at the beginning of the OMERO.table but well name and plate name at the very end? I typically use Cellprofiler output for uploading metadata, which has a lot of columns and therefore results into lots of scrolling to the right, if I want a readable well name, for example.

  5. Another side note for @will-moore and parade-crossfilter: when selecting one row of the table view panel of roi-based data, only one specific image is shown in the image preview panel (which is therefore fine). However, image-based data results in the preview of all fields of the wells, instead of a single image (which should be basically selected in the table view by selecting a single row).

I hope my notes are not too confusing and a bit helpful.

BR, Anna

will-moore commented 1 year ago

@abhamacher "Image Number" - do you mean the "Image" column? That is definitely supposed to be the Image ID. If it wasn't then the script is failing badly. Do you have some sample data (csv)?

I think omero-parade doesn't handle multiple Images per Well and certainly not multiple ROIs per Image.

omero-metadata always adds Columns at the end rather than inserting them.

I don't quite understand what 8) is about. Could you create a new issue for this on that repo (and same for any others above if you want, on their respective repos).

In the last commit above, I have reduced the logging level for warnings.

abhamacher commented 1 year ago

Hi Will,

I apologize for mixing up topics and bringing some confusion to this thread. As you can see, I added parts of the mentioned issues at the parade and parade-crossfilter repo. About the "ImageNumber" -- that was a mean one, it actually came from the original Cellprofiler data. Sorry, about that, so it has nothing to do with omero-metadata.

omero-metadata always adds Columns at the end rather than inserting them.

Well, still this is not very handy/ well readable for users. Especially because I add for example the plate and well names to the upload csv-file as the very first columns, but omero-metadata exchanges them to IDs and adds the names to the end of my table. Why can't the names just remain where they are (at the beginning) and the IDs being added to the end of the table?

Thanks, Anna

will-moore commented 1 year ago

Hi Anna, yes I agree that's annoying behaviour for HCS data (as I described in this PR description) - The behaviour for Project/Dataset/Image is better - in that case the "Image Name" column is unchanged and an "Image" ID column is simply added. I have previously created an issue for this at https://github.com/ome/omero-metadata/issues/29 Feel free to add comment there.

JensWendt commented 1 year ago

Hi @abhamacher ,

regarding point 5) --> I've been running into this issue also with other scripts. See my recent thread https://forum.image.sc/t/failed-to-get-results-error-while-running-scripts/76459 and the two other threads referenced in the first post there. It seems to be an issue with processes which take a lot of time or involve a lot of "entities", that at some point something goes wrong and you may or may not get the desired end result after seeing the "Failed to get results" statement.

snoopycrimecop commented 9 months ago

Conflicting PR. Removed from build OMERO-plugins-push#372. See the console output for more details. Possible conflicts:

--conflicts