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

ome 2021 workshop #88

Merged
merged 18 commits into from
Sep 2, 2021
Merged

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Apr 21, 2021

Adding setup info for OME2021 figure workshop.

To test:

Follow the steps in idr0079-data-prep.md.

@will-moore will-moore marked this pull request as ready for review April 21, 2021 17:06
@joshmoore
Copy link
Member

Not to block this PR, but I'll re-raise my concern here that as more logic gets added to these scripts, the more useful they become for other, non-training users. I'd really suggest that they get packaged in a re-usable way.

@will-moore
Copy link
Member Author

The most useful functionality here is the type of "move data from one server to another" (e.g. Map Annotations). I know users are interested in moving Images from one server to another, which is really the first step. To get Images from IDR for this workshop prep I'm using a script (just a gist) that I created for users at ome/omero-documentation#2179 (comment).
As discussed there, this uses our import.py which is still in gitlab (not released/supported?), so the first question is what to do with that?

There's other code in the training-scripts repo and this PR that is more IDR -> workshop specific (less useful to other users), but if we've already created a place for it to go then happy to include it.

First questions:

  • Do we add to omero-py (easier) or start another new repo?
  • Are we happy to include import.py code?

@joshmoore
Copy link
Member

I guess I would turn the questions back around: Are you ready to version the API with server? (If not, then omero-py probably isn't best)

Having an omero-import might be nice regardless, to include the CLI plugin. Some of these other functions might fall more readily under omero-annotations or similar.

@will-moore will-moore changed the title Add --ignore_datasets option to idr_get_map_annotations.py ome 2021 workshop features May 27, 2021
@will-moore will-moore changed the title ome 2021 workshop features ome 2021 workshop May 27, 2021
@joshmoore
Copy link
Member

Merging, since leaving this open as a PR isn't much of a better situation, but I'll re-iterate that I think we should work against the proliferation of one-off scripts.

@joshmoore joshmoore merged commit 6e4fdf1 into ome:master Sep 2, 2021
@will-moore
Copy link
Member Author

The masks_to_polygons.py is really a one-off for the figure workshop since figure needs Polygons but idr0079 has Masks. If we find other use-cases for that then we could consider putting it somewhere more accessible.
And the other new script is also very specific to idr0079, that works with the tsv files from idr0079 to create a table. Lots of values are hard-coded to work with idr0079 data and it really isn't useful elsewhere. The only reason it didn't get added to the IDR idr0079 repo is that it's summarising data specifically for OMERO.parade demo. The other idr0079 script got added to the IDR repo.

The functionality that channel_names_from_map_annotations.py provides has been discussed as being useful for IDR, but as I understand it, the channel names in existing map annotations from various studies are not consistent enough yet? cc @sbesson. The additional functionality in this PR was to transform the channels KVP to new Map-Annotations so as to be useful for OMERO.figure labels creation, which is also kinda one-off use-case.

I think of these scripts in the same way as scripts in each IDR repo: specific one-off scripts to perform a particular data preparation step.

Some of the changes I first made got reverted. For example, I realised that I could use metadata plugin to set pixel sizes.

@joshmoore
Copy link
Member

It's not as much about the scripts themselves (or the names) but the amount of code in them.

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.

2 participants