search

TheJacksonLaboratory / ezomero

A module with convenience functions for writing Python code that interacts with OMERO.
GNU General Public License v2.0
39 stars 13 forks source link

Map annotation enhancement; list of values possible #112

Closed JensWendt closed 3 months ago

JensWendt commented 4 months ago

Description

This tries to implement the option to pass a Key-Value dict containing multiple values for one key which then will create multiple Key-Value pairs in OMERO. Multiple instances of the key, each with one value. kv_dict = {"key1": ["value1", "value2"]} would become in OMERO:

key1 : value1
key1 : value2

The reason for this is, that I needed/wanted it, as the same functionality is also present in the new "KeyValue_from_csv" OMERO.web scripts. And it doesn't make sense to me, that the python package should have less functionality.

Checklist

It seems to me like I checked everything. But please double check, as I am not intimately familiar where I have to adapt things everywhere, as not to break things.

For reviewers

JensWendt commented 3 months ago

Heyo, I made the classic rookie mistake of pushing a PR and then going on holiday :) Just now answered your questions.

erickmartins commented 3 months ago

ok, with the benefit of a couple more days to think about this, I think I've flipped back to your original implementation. Most of the time there will be a single value per key, and for that more common use case we should stick to the main tenet of ezomero (a simple interface for the Python API), even if that makes maintenance marginally more difficult - so that should return a dict with string values, instead of always returning a list.

Apologies @JensWendt for the extra work and for the flip-flopping :) I've submitted a commit reverting things to your original implementation. Thanks a lot for the PR!

JensWendt commented 3 months ago

Okay, all good. I am happy that we get to implement it in any way. And with your reversion, it is not really extra work ^^ hurray for git versioning :D Just to make sure, it is now in a state that is okay for you and ready to merge at some point in the future? Or is there something else to improve?

erickmartins commented 3 months ago

Okay, all good. I am happy that we get to implement it in any way. And with your reversion, it is not really extra work ^^ hurray for git versioning :D Just to make sure, it is now in a state that is okay for you and ready to merge at some point in the future? Or is there something else to improve?

We should all be good to go - just gave it a day so you could see this was happening before I merged it :)