Apply inversion and quantization to proper channels #441

kkoz · 2023-01-30T18:46:35Z

Fixes #439. Refactors how inversions and quantizations are applied. In particular, pads out the quantizations so that if channels are missing from the c parameter, the corresponding channel has None as its quantization. Also, if the number of maps provided does not match the number of channels specified in c, a 400 is returned with an error message.

Testing

An IDR image is used here but any 3-channel image should work with the same query parameters. Similar to the issue, before the change the following URLs will yield different results, but with the change they should yield the same result:
https://idr.openmicroscopy.org/webgateway/render_image_region/14000763/0/0/?m=c&z=1&t=1&format=jpeg&c=-1|C0:255$FF0000,2|C0:255$00FF00,3|C0:255$0000FF&tile=1,2,3,1024,1024&maps=[{},{%22inverted%22:{%22enabled%22:true},%22quantization%22:{%22family%22:%22polynomial%22,%22coefficient%22:1.9}},{}]
https://idr.openmicroscopy.org/webgateway/render_image_region/14000763/0/0/?m=c&z=1&t=1&format=jpeg&c=2|C0:255$00FF00,3|C0:255$0000FF&tile=1,2,3,1024,1024&maps=[{%22inverted%22:{%22enabled%22:true},%22quantization%22:{%22family%22:%22polynomial%22,%22coefficient%22:1.9}},{}]

for more information, see https://pre-commit.ci

knabar

Functionality looks good to me, just some minor comments/questions

knabar · 2023-01-31T08:48:17Z

omeroweb/webgateway/views.py

@@ -2269,7 +2297,7 @@ def getRenderingSettings(image):
        return rv

    def applyRenderingSettings(image, rdef):
-        invert_flags = _get_maps_enabled(rdef, "inverted", image.getSizeC())
+        invert_flags = _get_inverted_enabled(rdef, "inverted", image.getSizeC())


Are the second and third argument still needed?

omeroweb/webgateway/views.py

knabar · 2023-01-31T08:55:54Z

omeroweb/webgateway/views.py

+        map_json = r["maps"]
+        # If coming from request string, need to load -> json
+        if isinstance(map_json, (unicode, str)):
+            map_json = json.loads(map_json)


In _get_inverted_enabled json.loads is wrapped in a try block to catch JSON errors, should this also be wrapped and return false on error?

… names

snoopycrimecop · 2023-02-01T00:36:53Z

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

PR Allow the JSONSerializer to be used as a session serializer #435 chris-allan 'Allow the JSONSerializer to be used as a session serializer'
- omeroweb/webgateway/views.py

--conflicts

for more information, see https://pre-commit.ci

jburel · 2023-02-01T11:01:23Z

omeroweb/webgateway/views.py:953:13: F821 undefined name 'log'
test/unit/test_webgateway.py:431:26: E712 comparison to True should be 'if cond is True:' or 'if cond:'
test/unit/test_webgateway.py:441:26: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
test/unit/test_webgateway.py:451:26: E712 comparison to False should be 'if cond is False:' or 'if not cond:'

will-moore · 2023-02-01T13:35:08Z

In the sample URLs from the description, first request has:

c=-1,2,3
maps=[{},{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]

2nd one has:

c=2,3
maps=[{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]]

Testing locally now, these give identical rendering settings (quantization is applied to 2nd channel which is also inverted)

?c=2,3&maps=[{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]
?c=-1,2,3&maps=[{},{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]

However, this doesn't throw any errors as expected when c and maps are different lengths:

?c=2,3&maps=[{},{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]

whereas if the maps is shorter than c:

?c=1,2,3&maps=[{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]
?c=-1,2,3&maps=[{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]

both give:

Traceback (most recent call last):
  File "/Users/wmoore/opt/anaconda3/envs/omeroweb/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/Users/wmoore/opt/anaconda3/envs/omeroweb/lib/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/wmoore/Desktop/WEB/omero-web/omeroweb/decorators.py", line 544, in wrapped
    retval = f(request, *args, **kwargs)
  File "/Users/wmoore/Desktop/WEB/omero-web/omeroweb/webgateway/views.py", line 1102, in render_image
    pi = _get_prepared_image(request, iid, server_id=server_id, conn=conn)
  File "/Users/wmoore/Desktop/WEB/omero-web/omeroweb/webgateway/views.py", line 917, in _get_prepared_image
    if not img.setActiveChannels(requestedChannels, windows, colors, invert_flags):
  File "/Users/wmoore/opt/anaconda3/envs/omeroweb/lib/python3.9/site-packages/omero/gateway/__init__.py", line 7955, in wrapped
    return f(self, *args, **kwargs)
  File "/Users/wmoore/opt/anaconda3/envs/omeroweb/lib/python3.9/site-packages/omero/gateway/__init__.py", line 8873, in setActiveChannels
    return self.set_active_channels(channels, windows, colors,
  File "/Users/wmoore/opt/anaconda3/envs/omeroweb/lib/python3.9/site-packages/omero/gateway/__init__.py", line 7955, in wrapped
    return f(self, *args, **kwargs)
  File "/Users/wmoore/opt/anaconda3/envs/omeroweb/lib/python3.9/site-packages/omero/gateway/__init__.py", line 8846, in set_active_channels
    invertMaps[idx] is not None):

…mage endpoint

… to match old behavior

…aps are provided than channels

snoopycrimecop · 2023-02-02T00:38:46Z

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

PR Allow the JSONSerializer to be used as a session serializer #435 chris-allan 'Allow the JSONSerializer to be used as a session serializer'
- omeroweb/webgateway/views.py

--conflicts

snoopycrimecop · 2023-02-02T00:48:29Z

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

PR Allow the JSONSerializer to be used as a session serializer #435 chris-allan 'Allow the JSONSerializer to be used as a session serializer'
- omeroweb/webgateway/views.py

--conflicts

omeroweb/webgateway/views.py

will-moore · 2023-02-02T18:08:45Z

Retesting URLs above, e.g. webclient/render_image/4472202/?c=2&maps=[{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]
now give the expected 400 error:

Must provide the same number of maps and channels or no maps

snoopycrimecop · 2023-02-03T00:38:40Z

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

PR Allow the JSONSerializer to be used as a session serializer #435 chris-allan 'Allow the JSONSerializer to be used as a session serializer'
- omeroweb/webgateway/views.py

--conflicts

snoopycrimecop · 2023-02-04T00:37:22Z

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

PR Allow the JSONSerializer to be used as a session serializer #435 chris-allan 'Allow the JSONSerializer to be used as a session serializer'
- omeroweb/webgateway/views.py

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

sbesson

Inline with the testing of glencoesoftware/omero-ms-image-region#120 (which motivated this PR), tested a few endpoints with and without this PR (using current IDR as the reference point)

a single resolution multi-channel image -https://idr.openmicroscopy.org/webclient/img_detail/10501752/ with different query parameters:
- ?tile=0,0,0,464,494 default
- ?m=c&c=1|0:10$00FF00,8|0:10$FF0000,13|0:10$0000FF,15|0:34.953$0000FF,26|0:10$FFFF00,30|0:10$00FFFF,34|0:10$FF00FF,41|0:10$FFFFFF&maps=[{},{},{},{},{},{},{},{"inverted":{"enabled":false},"quantization":{"family":"exponential","coefficient":2}}]&tile=0,0,0,464,494
- ?m=c&c=1|0:10$00FF00,8|0:10$FF0000,13|0:10$0000FF,15|0:34.953$0000FF,26|0:10$FFFF00,30|0:10$00FFFF,34|0:10$FF00FF,41|0:10$FFFFFF&maps=[{},{},{},{},{},{},{},{"inverted":{"enabled":false},"quantization":{"family":"polynomial","coefficient":2}}]&tile=0,0,0,464,494
- ?m=c&c=1|0:10$00FF00,8|0:10$FF0000,13|0:10$0000FF,15|0:34.953$0000FF,26|0:10$FFFF00,30|0:10$00FFFF,34|0:10$FF00FF,41|0:10$FFFFFF&maps=[{},{},{},{},{},{},{},{"inverted":{"enabled":false},"quantization":{"family":"logarithmic","coefficient":2}}]&tile=0,0,0,464,494
- ?m=c&c=1|0:10$00FF00,8|0:10$FF0000,13|0:10$0000FF,15|0:34.953$0000FF,26|0:10$FFFF00,30|0:10$00FFFF,34|0:10$FF00FF,41|0:10$FFFFFF&maps=[{},{},{},{},{},{},{},{"inverted":{"enabled":false},"quantization":{"family":"linear","coefficient":2}}]&tile=0,0,0,464,494
- ?m=c&c=1|0:10$00FF00,8|0:10$FF0000,13|0:10$0000FF,15|0:34.953$0000FF,26|0:10$FFFF00,30|0:10$00FFFF,34|0:10$FF00FF,41|0:10$FFFFFF&maps=[{},{"inverted":{"enabled":true}},{},{},{},{},{},{}]&tile=0,0,0,46
a multi-resolution image https://idr.openmicroscopy.org/webclient/img_detail/14000763/, with similar parameters as the one suggested in the description
- /?m=c&z=1&t=1&format=jpeg&c=-1|C0:255$FF0000,2|C0:255$00FF00,3|C0:255$0000FF&tile=1,20,3,1024,1024&maps=[{},{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]
- ?m=c&z=1&t=1&format=jpeg&c=2|C0:255$00FF00,3|C0:255$0000FF&tile=1,20,3,1024,1024&maps=[{"inverted":{"enabled":true},"quantization":{"family":"polynomial","coefficient":1.9}},{}]

Confirmed on current production IDR, the parameters are inconsistently interpreted and the rendered region differs from what is generated e.g. by adusting the rendering settings manually in the viewer. With this PR included, the behavior is working as expected.

The set of proposed changes make sense to me and ensure that the maps parameters has the same dimensions as the c parameters and that inversion/quantization values are assigned to the relevant channels.

More improvements/clarifications are being discussed in #443. Immediately, the PR fixes an outstanding issues and is ready for inclusion in the upcoming 5.18.0 release from my perspective.

kkoz and others added 6 commits January 30, 2023 12:12

Apply inversion and quantization to proper channels

c236656

Cleanup

6634214

[pre-commit.ci] auto fixes from pre-commit.com hooks

db90cb0

for more information, see https://pre-commit.ci

flake8

3cf0e10

merge

716f64c

[pre-commit.ci] auto fixes from pre-commit.com hooks

c70495c

for more information, see https://pre-commit.ci

sbesson requested review from sbesson, will-moore and jburel January 31, 2023 08:30

knabar reviewed Jan 31, 2023

View reviewed changes

kkoz added 3 commits January 31, 2023 16:55

Put json parsing in try block, fix function call, and update variable…

683a1a0

… names

Merge branch 'maps-fix' of github.com:kkoz/omero-web into maps-fix

59f9fd8

unicode and str are the same in python3

77e45f6

kkoz and others added 2 commits January 31, 2023 21:09

Validate False if maps json malformed

b0c2a96

[pre-commit.ci] auto fixes from pre-commit.com hooks

ac89af4

for more information, see https://pre-commit.ci

knabar mentioned this pull request Feb 1, 2023

Add change log for 5.18.0 #442

Merged

kkoz added 4 commits February 1, 2023 10:06

Fix test comparison

7dcb28f

Fix formatting and test comparison syntax

1f0b50b

More logging when setQuantizationMaps fails and validate for render_i…

d73c585

…mage endpoint

Add more maps validation checks and pad inversions with None to sizeC…

b5c23a9

… to match old behavior

kkoz force-pushed the maps-fix branch from ea07ce8 to e257280 Compare February 1, 2023 22:07

Validate for saving and revert _get_prepared_image to work if fewer m…

34139a0

…aps are provided than channels

kkoz force-pushed the maps-fix branch from e257280 to 34139a0 Compare February 1, 2023 22:09

Fix tests

1fc22db

sbesson reviewed Feb 2, 2023

View reviewed changes

omeroweb/webgateway/views.py Show resolved Hide resolved

sbesson mentioned this pull request Feb 3, 2023

Consistent Behavior for Rendering Settings Query Parameters #440

Open

Merge JSONSerializer work

dc334ea

sbesson self-requested a review February 13, 2023 16:04

sbesson approved these changes Feb 14, 2023

View reviewed changes

knabar merged commit 9ac7ade into ome:master Feb 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply inversion and quantization to proper channels #441

Apply inversion and quantization to proper channels #441

kkoz commented Jan 30, 2023 •

edited

knabar left a comment

knabar Jan 31, 2023

knabar Jan 31, 2023

snoopycrimecop commented Feb 1, 2023

jburel commented Feb 1, 2023

will-moore commented Feb 1, 2023

snoopycrimecop commented Feb 2, 2023

snoopycrimecop commented Feb 2, 2023

will-moore commented Feb 2, 2023

snoopycrimecop commented Feb 3, 2023

snoopycrimecop commented Feb 4, 2023 •

edited

sbesson left a comment

Apply inversion and quantization to proper channels #441

Apply inversion and quantization to proper channels #441

Conversation

kkoz commented Jan 30, 2023 • edited

Testing

knabar left a comment

Choose a reason for hiding this comment

knabar Jan 31, 2023

Choose a reason for hiding this comment

knabar Jan 31, 2023

Choose a reason for hiding this comment

snoopycrimecop commented Feb 1, 2023

jburel commented Feb 1, 2023

will-moore commented Feb 1, 2023

snoopycrimecop commented Feb 2, 2023

snoopycrimecop commented Feb 2, 2023

will-moore commented Feb 2, 2023

snoopycrimecop commented Feb 3, 2023

snoopycrimecop commented Feb 4, 2023 • edited

sbesson left a comment

Choose a reason for hiding this comment

kkoz commented Jan 30, 2023 •

edited

snoopycrimecop commented Feb 4, 2023 •

edited