search

ome / omero-scripts

Core OMERO Scripts
https://pypi.org/project/omero-scripts/
12 stars 32 forks source link

populate metadata links file ann #152

Closed will-moore closed 4 years ago

will-moore commented 5 years ago

This change is needed to allow populate_metadata script to work with the File-picker from https://github.com/ome/openmicroscopy/pull/6096 That PR passes the uploaded FileAnnotation ID to populate-metadata script, but the script expects the FileAnnotation to be linked to the selected object. This PR links the FileAnnotation to the object, which also has the benefit of not 'losing' the new FileAnnotation with it being unlinked.

To test:

jburel commented 5 years ago
will-moore commented 5 years ago

It looks like merge-ci doesn't have the latest version of the script, although it was merged in the last build at https://github.com/snoopycrimecop/scripts/commit/b7de9d77ffc8264001c9fecd2498474e4f094b15

sbesson commented 5 years ago

I was surprised by this at first it looks consistent to the introduction of the --shallow flag in https://github.com/ome/devspace/commit/9a99bf129029c867904e0abaf1fd90ae2a4751d3.

will-moore commented 5 years ago

Does that commit need reverting, or some other fix?

sbesson commented 5 years ago

Reverting the commit will only apply to newly deployed devspace environment. In the case of the existing merge-ci devspace, I think you want to modify the default value of MERGE_COMMAND in the job configuration and restore the previous behavior unless @jburel remembers a reason not to revert his change.

jburel commented 5 years ago

I do not remember why we added the shallow flag

joshmoore commented 5 years ago

For what it's worth, we will very soon no longer have a submodule in ome/openmicroscopy.

joshmoore commented 5 years ago

Coming back to the content of the PR:

This change is needed to allow populate_metadata script to work with the File-picker from ome/openmicroscopy#6096

We might need to discuss how this type of coupling should affect the semver of these two components.

will-moore commented 5 years ago

Certainly we shouldn't release the change in the script dialog https://github.com/ome/openmicroscopy/pull/6096 before the script changes in this PR.

There are a few places where we have a 'convention' on the naming of script Parameters. E.g. "Data_Type" and "IDs" for data input. These are quite simple convention so we haven't felt the need to define this API anywhere. The "File_Annotation" parameter is also understood by the webclient and can be auto-populated. I assumed File_Annotation would simply use the ID of a File_Annotation, but it has an additional requirement that the Annotation is linked to the Project/Screen for populate_metadata script.

Perhaps we can test and merge this PR first, then consider https://github.com/ome/openmicroscopy/pull/6096

Will that be enough to ensure that user don't get the webclient dialog changes before they get the script changes?

cc @chris-allan

pwalczysko commented 5 years ago

We still do not have this script on the merge-ci - judging according to me getting the same result of the test as @jburel above https://github.com/ome/scripts/pull/152/#issuecomment-522608879 - how to improve that ?

pwalczysko commented 5 years ago

Uploaded the script here manually onto merge-ci. The functionality works as described.

will-moore commented 4 years ago

Closing until #159 is merged to avoid conflicts...

pwalczysko commented 4 years ago

All works fine here. Tried the suggested table as well as my own, with double columns -> parade

The unattached File is found via the ID as expected, and when one is attached there already, the script still runs as expected.

Ready to merge FMPOV

pwalczysko commented 4 years ago

Maybe we should test on py3-ci though ? All the tests above were run on merge-ci

will-moore commented 4 years ago

Uploaded and tested on py3-ci today. Will list for testing there tomorrow (will need to upload script again first). Use https://py3-ci.openmicroscopy.org/web/webclient/?show=dataset-3120 (user-3).

pwalczysko commented 4 years ago

Tested on py3-ci, user-3 https://py3-ci.openmicroscopy.org/web/webclient/?show=dataset-2270

All good. Ready to merge.

joshmoore commented 4 years ago

This change is needed to allow populate_metadata script to work with the File-picker from ome/openmicroscopy#6096

But that PR is excluded. Can you explain?

will-moore commented 4 years ago

https://github.com/ome/openmicroscopy/pull/6096 requires this PR, so it is probably excluded until this one is merged, released etc. However, this PR doesn't require that one.