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

Obj id bitmask endpoint #308

Merged
merged 24 commits into from
Nov 9, 2021
Merged

Conversation

kkoz
Copy link
Contributor

@kkoz kkoz commented Aug 5, 2021

This PR adds an endpoint to make label image data retrieval more efficient from omero tables. Rather than returning the entire result of a query which could be a very large number of objects, the endpoint returns a bitmask representing the object IDs, where the bit is set to 1 if that ID matched the query and 0 otherwise. You can hit the endpoint using a URL like http://localhost/webgateway/table/123/obj_id_bitmask/?query=value<14. A col_name parameter can also be passed if your object ID column name is not named object

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-python-superbuild-push#779. See the console output for more details.
Possible conflicts:

--conflicts

data = bytearray()
for obj in rsp_data["data"]["rows"]:
obj_id = obj[0]
while index < obj_id:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my case, the first column in this row is a 'StringColumn', so that obj_id is a string (and this comparison line fails).

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-python-superbuild-push#783. See the console output for more details.
Possible conflicts:

--conflicts

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-python-superbuild-push#786. See the console output for more details.
Possible conflicts:

--conflicts

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OMERO-python-superbuild-push#787. See the console output for more details.
Possible conflicts:

--conflicts

@kkoz
Copy link
Contributor Author

kkoz commented Aug 17, 2021

@will-moore Is there something I can or should do on this PR to resolve SCC's conflict comments?

@will-moore
Copy link
Member

Apologies: I meant to ping @chris-allan and @erindiel for final feedback on those (#270 and #300) but I think they should be close to getting merged.
In the meantime, I can test this PR again locally...

@will-moore
Copy link
Member

Testing locally with a table where this URL /webgateway/table/68129/query/?query=Count<400 gives me...

this JSON
{
"data": {
    "column_types": [
        "StringColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "DoubleColumn",
        "StringColumn",
        "DoubleColumn",
        "DoubleColumn",
        "ImageColumn"
    ],
    "columns": [
        "Slice",
        "Count",
        "Total_Area",
        "Average_Size",
        "%Area",
        "Mean",
        "Mode",
        "Perim.",
        "Feret",
        "FeretX",
        "FeretY",
        "FeretAngle",
        "MinFeret",
        "Dataset",
        "Bounding_Box",
        "Channel_Index",
        "Image"
    ],
    "rows": [
        [
            "c:1/2 - PCNT_gtub_I_044_SIR_PRJ.dv",
            200,
            74.26790595054626,
            37.13395297527313,
            72.6318359375,
            0,
            0,
            341.06953144073486,
            92.94512271881104,
            12900,
            11050,
            8191.774749755859,
            62.0566189289093,
            "PCNT-N1",
            177.86849737167358,
            100,
            1101
        ],
        [
            "c:2/2 - N20_PCNTab_ctrl_20110618_Sat-1400_0_SIR_PRJ.dv",
            200,
            46.88798189163208,
            23.44399094581604,
            45.623779296875,
            0,
            0,
            197.2864270210266,
            63.870203495025635,
            10800,
            10750,
            13633.149719238281,
            55.110132694244385,
            "PCNT-N1",
            42.66819953918457,
            200,
            1091
        ],
        [
            "c:1/2 - PCNT_gtub_I_010_SIR_PRJ.dv",
            300,
            55.70092797279358,
            18.566976487636566,
            54.473876953125,
            0,
            0,
            145.4495906829834,
            52.599912881851196,
            13933.332824707031,
            12633.333587646484,
            4308.696746826172,
            32.88620710372925,
            "PCNT-N1",
            85.24099588394165,
            100,
            1106
        ],
        ...

When I use: /webgateway/table/68129/obj_id_bitmask/?query=Count<400&col_name=Image I get:

{
"message": "byte must be in range(0, 256)",
"stacktrace": "Traceback (most recent call last):\n  File \"/Users/wmoore/Desktop/WEB/omero-web/omeroweb/webgateway/views.py\", line 1431, in wrap\n    rv = f(request, *args, **kwargs)\n  File \"/Users/wmoore/Desktop/WEB/omero-web/omeroweb/webgateway/views.py\", line 3125, in obj_id_bitmask\n    data.append(value)\nValueError: byte must be in range(0, 256)\n"
}

So either I'm not understanding the usage or my data is not as expected?

@snoopycrimecop
Copy link
Member

snoopycrimecop commented Aug 18, 2021

Conflicting PR. Removed from build OMERO-python-superbuild-push#790. See the console output for more details.
Possible conflicts:

--conflicts Conflict resolved in build OMERO-python-superbuild-push#792. See the console output for more details.

@will-moore
Copy link
Member

So, the URL I was testing above /webgateway/table/68129/obj_id_bitmask/?query=Count<400&col_name=Image is now downloading something.
Do you have any sample code to help me validate whether this is correct? thx.

@kkoz
Copy link
Contributor Author

kkoz commented Aug 23, 2021

@will-moore I've put a test script I wrote here: https://gist.github.com/kkoz/791555afe60bc8dc6a4cd6c3c57988ed
Run this script against a table with an 'object' column and you should get back a bitmask where the bit is set to 1 if the object ID is returned by your query or 0 otherwise. E.g. if you had object IDs 0, 1, 3, 9, 22, and 23 and used the query object < 10 you should get back 1101000001000000 (there will probably be some extra 0s at the end). Let me know if that works for you

@will-moore
Copy link
Member

I'm not sure if I understand your example:

>>> ids = [0, 1, 3, 9, 22, 23]
>>> ''.join(["1" if id < 10 else "0" for id in ids])
'111100'

not 1101000001000000 (even with some padding)?

I'm adding a test which should help us run the same tests.
I didn't manage to get your gist working (kept getting the login page returned - maybe using wrong sessionid) but the code sample was useful for writing a test.

Test added at ome/openmicroscopy#6289

@kkoz
Copy link
Contributor Author

kkoz commented Aug 24, 2021

@will-moore The sessionid is supposed to be a omero web session ID, the one stored as the 'sessionid' cookie in the browser. Sorry I should have specified that.

Copy link
Member

@chris-allan chris-allan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is going into the public API at a minimum we need an extensive docstring in urls.py describing what the intent is here and what the caller can be expected to receive. Things like endianness, padding, and the layout of the bytes need to be explicitly defined.

Furthermore, some unit tests at least enforcing the byte layout need to be added. This probably necessitates refactoring the logic for producing the resulting bytes out of the view method.

omeroweb/webgateway/views.py Outdated Show resolved Hide resolved
@will-moore
Copy link
Member

Updated the test at will-moore/openmicroscopy@8b0991d to correspond with the usage of numpy.packbits() above.

@chris-allan
Copy link
Member

@will-moore: Anything else from your side that you think we need to do before getting this merged?

/cc @sbesson, @jburel

@sbesson
Copy link
Member

sbesson commented Nov 4, 2021

@chris-allan could you also update your previous review #308 (review).

No objections to getting this merged from my side. I have two follow-up questions regarding the versioning and distribution:

  • I assume the development version number could be immediately incremented to 5.12.0.dev0 to communicate that a new backwards compatible API and endpoint have been introduced in the main development line
  • should this feature be released immediately or can we define a roadmap towards OMERO.web 5.12.0?

@chris-allan
Copy link
Member

I assume the development version number could be immediately incremented to 5.12.0.dev0 to communicate that a new backwards compatible API and endpoint have been introduced in the main development line

Certainly.

should this feature be released immediately or can we define a roadmap towards OMERO.web 5.12.0?

Unless there's a really compelling reason not to, from our side it would be really nice to have this released.

@will-moore will-moore added this to the 5.12.0 release milestone Nov 5, 2021
@will-moore will-moore merged commit c015afb into ome:master Nov 9, 2021
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.

None yet

6 participants