Skip to content

Populate metadata#107

Merged
jburel merged 12 commits intoome:developfrom
will-moore:Populate_Metadata
Feb 25, 2016
Merged

Populate metadata#107
jburel merged 12 commits intoome:developfrom
will-moore:Populate_Metadata

Conversation

@will-moore
Copy link
Copy Markdown
Member

As discussed ome/openmicroscopy#4455 (comment) Now that we support visualisation of bulk annotations in web (and Insight via tooltip) we want to make it easier for users to attach bulk annotations.

This PR copies the Glencoe script from https://github.com/glencoesoftware/omero-user-scripts/blob/master/util_scripts/Populate_Metadata.py and updates it a bit:

  • Add (hopefully) useful description
  • Use File_Annotation ID for picking files instead of File ID (web and Insight UI allow for picking File Ann)
  • Make File_Annotation optional - If not set, we simply pick a .csv file from the Plate

To test, use Plate and csv files described https://github.com/openmicroscopy/ome-internal/blob/master/testing_scenarios/BulkAnnotations.txt

  • Import Plate, attach test_plate.csv to it
  • Select Plate and run script > Import Scripts > Populate_Metadata
  • Read description. Makes sense?
  • Run (don't need to choose File Annotation - will pick the csv automatically).
  • Also, if multiple csv attached, you can pick the one you want from right panel and this will be used to populate the "File Annotation" parameter.
  • When the script completes, selecting Wells should show the Bulk Annotation in right panel (as in Bulk annotations right panel openmicroscopy#4446).

screen shot 2016-02-04 at 15 27 54

screen shot 2016-02-04 at 15 26 12

@will-moore
Copy link
Copy Markdown
Member Author

cc @emilroz I've copied your script here and made a few changes (see above).

@pwalczysko
Copy link
Copy Markdown
Member

The output on the right-hand side is a bit repetitive. Not sure why. I have admittedly ran the script twice (deleting the Bulk_annotation attachment in between of the two script executions) but still, some lines seem to be there 4 times and more.
screen shot 2016-02-05 at 12 01 22

@pwalczysko
Copy link
Copy Markdown
Member

Re: #107 (comment) Clarified with @will-moore that the duplication came from the fact that there was another ``Bulk_annotation` attachment on the containing Screen left from previous testing. When this attachment was deleted, the duplication vanished.

This poses an important question of what to do when there is multiple exemplaries of the attachment -> tactics might differ (even between Web and Insight), but definitely the display should be separate for each attachment in the right-hand pane display, similarly to the Key-Value pairs.

@pwalczysko
Copy link
Copy Markdown
Member

screen shot 2016-02-05 at 12 31 47

I think a hint about how do I construct the csv in case it is attached to a screen would be very useful in the description of the script (see above).
This screen workflow works for me having following .csv (see screenshot below) where the Plate column actually contains the name of the plate in OMERO (it is just misformatted by Excel as a number, see second screenshot below)

screen shot 2016-02-05 at 12 36 58
screen shot 2016-02-05 at 12 38 58

@emilroz
Copy link
Copy Markdown
Member

emilroz commented Feb 8, 2016

@will-moore the script looks good to me.
@pwalczysko I think that an example .csv file would be better then a very long description. As you've noticed the formatting plays also very important role here, as we are really only prepared now to take properly formatted comma separated value files anything else will fail silently - you will get a single column table as an output and as a result no bulk annotations on the images.

@pwalczysko
Copy link
Copy Markdown
Member

@emilroz : Agree, an example file is better. How shouild the user find such a file ?

@joshmoore
Copy link
Copy Markdown
Member

Will a link to a file be clickable in web & insight? If so, I'd think a link to a sample image (perhap a fake?) which includes a CSV with a readme on how to install it would be useful?

cc: @sbesson @simleo @manics -- This would also be a first candidate for https://trello.com/c/9aEYto1f/217-table-import-prototype

@will-moore
Copy link
Copy Markdown
Member Author

@joshmoore Apologies for the delay. Unfortunately web scripts dialog doesn't make links clickable at the moment. However, I think a non-clickable link to proper docs and example csv is still the best option.
We just need to get docs in place so we have the url. Don't know if we can do this in time for this PR: @gusferguson?

@gusferguson
Copy link
Copy Markdown

@will-moore @joshmoore - what were you thinking in terms of User Help documentation? Easy enough to have an example doc in the resources part of the User Help - that will give an immutable URL for downloads and whatever/wherever there is something in the actual User Help.

@joshmoore
Copy link
Copy Markdown
Member

I'll leave you guys to discuss what's doable for 5.2.2. We can capture any extras in a card for later.

NB: @will-moore, sorry for having missed it earlier, could you possibly rebase this onto of joshmoore@47cd2ff (i.e. replacing your 7298da8 with commits from @emilroz so that we have the history. (I've included joshmoore@a0dd392 on top to do the rename unless you want to do that yourself)

@gusferguson
Copy link
Copy Markdown

@will-moore @joshmoore
Confirmed that I will be putting the Help for this script in the Using Scripts page with a #metadata anchor, so the URL:
http://help.openmicroscopy.org/scripts.html#metadata
will be the one to use.

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 22, 2016

The script does not follow the approach used for ids in other scripts i.e. ids as rlist not rstring
This was the source of the error in insight ome/openmicroscopy#4487
and it also means that the parsing will now happen in the script itself if multiple ids are passed

cc @will-moore @emilroz

@gusferguson
Copy link
Copy Markdown

Has this been tested with Insight?
I am getting errors trying to run it on eel using today's build.
Works fine in Web.

@gusferguson
Copy link
Copy Markdown

Actually not running in web now.

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 22, 2016

@emilroz: are you okay if we change the type for IDs i.e. list instead of String.
This will require some re-tests of the scripts
The only limitation is that the "FileAnnotation" option will potentially not work when multiple objects are selected (unless it is linked to all the objects)

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 22, 2016

@gusferguson: insight PR is ome/openmicroscopy#4487. I pushed a fix this am (not included in the build)

@gusferguson
Copy link
Copy Markdown

@jburel +1

@will-moore
Copy link
Copy Markdown
Member Author

Updated the URL in the description as discussed.
Also switched to using List for IDs. We only support a single ID. Warning is printed to output if multiple IDs are given - we just use the first.
Added a response message to display when the script completes.

To test, select multiple Screens or Plates. Should see warning in Info when complete.
Also if completes, should see a simple message:

screen shot 2016-02-22 at 23 26 02

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 23, 2016

Message displayed if multiple ids selected:
for plate

{'IDs': [226L, 8L], 'Data_Type': 'Plate'}
WARNING: Multiple IDs not currently supported
    Only using the first ID: 226
Listing files on Plate 226...

for screen

{'IDs': [801L, 1151L], 'Data_Type': 'Screen'}
WARNING: Multiple IDs not currently supported
    Only using the first ID: 801
Listing files on Screen 801...

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 23, 2016

Log in as user-4, screen Bulk_Annotation_test
WARNING:omero.util.populate_metadata:PlateColumn is unimplemented

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 23, 2016

Warning introduced in joshmoore/openmicroscopy@00d89c6
cc @joshmoore
Web indicated that the result has been created. No message in insight only the error button.
This will have to be improved post release in insight

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 23, 2016

The script works as expected now. I will retest of insight w/o ome/openmicroscopy#4487 included (changes no longer required)

@emilroz
Copy link
Copy Markdown
Member

emilroz commented Feb 23, 2016

@jburel I'm fine with changing the parameter type.

Edit: added fine :)

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 24, 2016

@emilroz: Thanks, that's the best option. I have closed my insight PR.

@will-moore
Copy link
Copy Markdown
Member Author

@joshmoore That's the rebasing done that you asked for in #107 (comment), including the renaming commit. I checked that the script is unchanged from before, (we just have the commit history) so it doesn't need retesting.

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 24, 2016

Tested w/o insight changes included, it works fine.

pwalczysko referenced this pull request in hflynn/openmicroscopy Feb 25, 2016
jburel added a commit that referenced this pull request Feb 25, 2016
@jburel jburel merged commit 456da9c into ome:develop Feb 25, 2016
@joshmoore
Copy link
Copy Markdown
Member

Note: without the git mv this is now the only script we ship outside of omero.

joshmoore added a commit to joshmoore/omero-scripts that referenced this pull request Feb 25, 2016
Follow-on to omegh-107 with a renaming based on the standard
ome/scripts scheme.
@will-moore
Copy link
Copy Markdown
Member Author

Sorry @joshmoore I had cherry-picked your move commit locally but hadn't pushed it. Done now.
Ooops - I see it's just been merged already.

@will-moore
Copy link
Copy Markdown
Member Author

I'll open a new PR with the move commit.

* 1155250 (HEAD -> Populate_Metadata, gh/Populate_Metadata) Rename from util_scripts

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 25, 2016

@will-moore: A PR has now been opened.

@will-moore
Copy link
Copy Markdown
Member Author

OK - e-mail delay is annoying! Closed #110

@joshmoore
Copy link
Copy Markdown
Member

Sorry, @will-moore. I should have commented here (I had even thought about it!)

@sbesson sbesson added this to the 5.3.0 milestone Apr 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants