Skip to content

Add get subcommand to CLI obj command#4071

Merged
joshmoore merged 5 commits intoome:dev_5_1from
ximenesuk:cli-obj-get
Sep 7, 2015
Merged

Add get subcommand to CLI obj command#4071
joshmoore merged 5 commits intoome:dev_5_1from
ximenesuk:cli-obj-get

Conversation

@ximenesuk
Copy link
Copy Markdown
Contributor

See https://trello.com/c/5M0gWeCw/43-omero-obj-new-commands This addresses just the first bullet point.

omero obj get Object:XX field

should return the value of a simple field, name, description, etc. More complex fields should cause an error.

The one modified test, which checks one use of get should pass.

--rebased-to #4141

@manics
Copy link
Copy Markdown
Member

manics commented Aug 20, 2015

At present this PR is a place to hang some discussion.

Opening a can of worms!

Here's an example of the output I've been printing for annotations in the omero metadata plugin (#4037 #4062) in case that helps the discussion.

$ OMERO-TEST/bin/omero metadata allanns Image:23451 --parents
MapAnnotation:6812
FileAnnotation:6809
FileAnnotation:6810
FileAnnotation:6811
$ OMERO-TEST/bin/omero metadata allanns Image:23451 --parents --report
Image:23451
  WellSample:20116
    Well:8259
      MapAnnotation:6812
        ns: openmicroscopy.org/omero/bulk_annotations
        description:
        date: 2015-08-19T18:27:11
        value:
          Primary Compound URL=https://www.google.co.uk/search?q=BI-2536&gws_rd=cr
          __Primary Compound Name=BI-2536
          Well=8259
          Well Type=Treatment
          Primary Compound Name=BI-2536
          Facility-Salt-Batch-ID=FOOL10041-101-2
          Library=FOO-BAR Anti-Mitotics Plate 6
          Reagent Vendor=Bubba Lab
          concentration (uM)=3333.333333
          final well conc (nM)=11111.11111
          Well Name=a1
      Plate:501
        FileAnnotation:6809
          ns: openmicroscopy.org/omero/bulk_annotations
          description: bulk_annotations
          date: 2015-08-19T18:25:56
          file: OriginalFile:126298
            name: bulk_annotations
            size: 75056
        FileAnnotation:6810
          ns: openmicroscopy.org/omero/measurement
          description: NEOlog2008-09-18-14h37m07s
          date: 2015-08-19T18:26:13
          file: OriginalFile:126299
            name: /NEOlog2008-09-18-14h37m07s.r5
            size: 75168
        FileAnnotation:6811
          ns: openmicroscopy.org/omero/bulk_annotations/config
          description:
          date: 2015-08-19T18:27:10
          file: OriginalFile:126300
            name: config.yml
            size: 1362
        Screen:501
$ OMERO-TEST/bin/omero metadata summary Image:101
Image:101
---------
Name: Screen Shot 2014-09-02 at 11.50.59.png
Roi count: 1
Bulk annotations: 0
Parent: Dataset:101
Source metadata: False
Global metadata: False
Series metadata: False
CommentAnnotations: 2
FileAnnotations: 1
MapAnnotations: 1
TagAnnotations: 1

@ximenesuk
Copy link
Copy Markdown
Contributor Author

I am tempted here to drop support for:

omero obj get Object:XX

within this PR and asscope and, later, add

omero obj getlist Object:XX

as a separate command?

@manics
Copy link
Copy Markdown
Member

manics commented Aug 25, 2015

That's probably the least contentious option.

@manics manics mentioned this pull request Aug 25, 2015
@ximenesuk
Copy link
Copy Markdown
Contributor Author

Closing this temporarily while I'm away as it needs slight revision and I don't want it to interfere with any other work.

@ximenesuk ximenesuk closed this Aug 26, 2015
@ximenesuk ximenesuk reopened this Aug 26, 2015
@ximenesuk
Copy link
Copy Markdown
Contributor Author

The originally included usage of

omero obj get Object:XX

for listing the fields of an object has been dropped from this PR. We whould introduce a getlist subcommand for that purpose when we know what is needed.

@joshmoore
Copy link
Copy Markdown
Member

As discussed elsewhere: a group owner or admin should be able to modify ExperimenterGroup.config and anyone should be able to read it.

We would introduce a getlist subcommand for that purpose when we know what is needed.

Thinking about breaking changes, my gut feeling is that we should steer toward list-[gs]et earlier rather than later.

@ximenesuk
Copy link
Copy Markdown
Contributor Author

There may be some confusion here as to what the getlist referred to above was intended to do. From the original descripion on this PR the intention was to return a list of the fields for a given object - something @manics thought might be useful. This is the bit that I was suggesting be deferred as which fields exactly to return is very much object dependent.

Looking at the Trello card list-get as specified there (it was originally just get) is effectively what is happening here at the moment with get when an index is provided.

However, if we want a fuller list-get (and list-update/list-append/etc.) that does more than get the value of the field at the index - handling whole lists? - then it may be worth elaborating on the Trello card? (And closing this PR for now?)

@joshmoore
Copy link
Copy Markdown
Member

  • getlist: yeah, let's defer that then, and likely change the name (info, schema, definition, fields, etc)
  • get v. list-get: my major concern is that we don't tie our hands too much with changes to the get API. If it's a new command (e.g. list-get) then we're free to later adapt get further.
  • for the likes of list-update, -append etc. I'd definitely say, "later". (There's still a question of should some command return all the values in a list, too)

@ximenesuk
Copy link
Copy Markdown
Contributor Author

Bear in mind there are no changes to the get API, it is a completely new subcommand introduced by this PR

A proposal:

omero obj get Object:XX field

gets the named field whether it is an individual value or a list. A list is returned as a CSV.

omero obj get Object:XX field --index Y

get the element at Y in the list. If not a list or no element exists then returns an error.

With the later addition of obj update taking CSV input to update list fields this would then allow (initially at least) appending, sorting, etc to be done using a get/update script.

Later additions could then be --index to update to set individual elements, --append to add element(s) to the end of existing list fields, --sort [ascending|descending] or such like.

@joshmoore
Copy link
Copy Markdown
Member

Agreed. And if we need, we can SUPPRESS --index later if scoping shows that something like list-get or whatever would be more appropriate.

@ximenesuk
Copy link
Copy Markdown
Contributor Author

I'm now minded to leave this PR as it stands. It gets basic fields that return values and throws an error otherwise. Looking at some of the lists they are lists of named values and are dealt with via the map commands. I would suggest that any further work on dealing with lists that do not represent map annotations is scoped with examples of which fields are applicable and how values should be returned.

@joshmoore
Copy link
Copy Markdown
Member

Works for me. When/if we go for the more complicated version, one further improvement might be to make get etc. proper argparse subcommand, so that individual help is available.

https://ci.openmicroscopy.org/view/Failing/job/OMERO-5.1-merge-integration-python/545/testReport/ passed for these tests. Merging.

joshmoore added a commit that referenced this pull request Sep 7, 2015
Add get subcommand to CLI obj command
@joshmoore joshmoore merged commit be39fee into ome:dev_5_1 Sep 7, 2015
@ximenesuk
Copy link
Copy Markdown
Contributor Author

Yes, being proper subcommands would help. When I briefly had --index in there it was exposed for all commands and so needed qualifying more than it should.

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.

5 participants