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

Always marshal tile metadata on presence of pyramid #239

Merged
merged 1 commit into from Sep 10, 2020

Conversation

chris-allan
Copy link
Member

It is possible, in particular for floating point data, that a pyramid is
present even if the server thinks a pyramid is not required. The
condition for tile metadata being available is now pyramid presence.

See:

It is possible, in particular for floating point data, that a pyramid is
present even if the server thinks a pyramid is not required.  The
condition for tile metadata being available is now pyramid presence.

See:

 * ome/omero-romio#24
 * ome/omero-web#193
@chris-allan
Copy link
Member Author

Additional requirement for restoring all the pyramid metadata that was started in ome/omero-web#193.

/cc @sbesson

@joshmoore
Copy link
Member

joshmoore commented Jul 31, 2020

The change seems straight-forward enough, though I imagine we should clarify in https://github.com/ome/omero-blitz/blob/master/src/main/slice/omero/api/PyramidService.ice when each of the methods should be used.

@chris-allan
Copy link
Member Author

@joshmoore: Yeah, in particular requirePixelsPyramid() as currently documented is already largely incorrect:

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

Tested with the same sample floating-point multi-resolution image as the one noted in ome/omero-web#193

Without this PR, the marshalled image data for this image is:

 "tiles": true,
  "tile_size": {
    "width": 1024,
    "height": 1024
  },
  "levels": 5,
  "interpolate": true,
  "size": {
    "width": 7676,
    "height": 7420,
    "z": 38,
    "t": 1,
    "c": 1
  },
  "pixel_size": {
    "x": 0.00006375423073768616,
    "y": 0.00006375423073768616,
    "z": 1
  },
  "init_zoom": 0,
  "nominalMagnification": 105000,
  "pixel_range": [
    -2147483648,
    2147483647
  ],

With this PR included

  "tiles": true,
  "tile_size": {
    "width": 1024,
    "height": 1024
  },
  "levels": 5,
  "zoomLevelScaling": {
    "0": 1,
    "1": 0.5,
    "2": 0.25,
    "3": 0.12493486190724336,
    "4": 0.06240229286086504
  },
  "interpolate": true,
  "size": {
    "width": 7676,
    "height": 7420,
    "z": 38,
    "t": 1,
    "c": 1
  },
  "pixel_size": {
    "x": 0.00006375423073768616,
    "y": 0.00006375423073768616,
    "z": 1
  },
  "init_zoom": 0,
  "nominalMagnification": 105000,
  "pixel_range": [
    -2147483648,
    2147483647
  ],

So the proposed change is functional, is inline with the fix already released in the latest omero-web patch release.

While reviewing #239 (comment), I also found the getResolutionLevels docstring includes a caveat about the usage of this API https://github.com/ome/omero-blitz/blob/master/src/main/slice/omero/api/PyramidService.ice#L43. Any thought on whether this limitation is still applicable and should change the beheavior of this gateway API? If no longer valid, it can probably be reviewed together with the documentation of requiresPixelsPyramid.

Once the question above is cleared, proposing to get this merged for inclusion in the upcoming omero-py minor release.

@sbesson sbesson merged commit 6c86a93 into ome:master Sep 10, 2020
manics added a commit to manics-archive/omero-py that referenced this pull request Sep 10, 2020
Co-authored-by: Sébastien Besson <seb.besson@gmail.com>
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

3 participants