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

Allow a caller to ignore row numbers #418

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

chris-allan
Copy link
Member

For calls to readCoordinates, read, and slice the returned value order in the Data response is the same as requested. While having the row numbers included in the response is convenient, when the number of cells being returned is high this incurs memory and serialization overhead. This is especially true when retrieving a small number of columns for a large number of rows; in this case rowNumbers can actually be more expensive to include than the data itself.

Here we use the Ice context and "omero.tables.include_row_numbers" to additively affect the client API without changing any of the Ice method prototypes.

/cc @erindiel, @kkoz, @DavidStirling, @emilroz

For calls to readCoordinates, read, and slice the returned value order
in the `Data` response is the same as requested.  While having the row
numbers included in the response is convenient, when the number of cells
being returned is high this incurs memory and serialization overhead.
This is especially true when retrieving a small number of columns for a
large number of rows; in this case `rowNumbers` can actually be more
expensive to include than the data itself.

Here we use the Ice context and "omero.tables.include_row_numbers" to
additively affect the client API without changing any of the Ice method
prototypes.
@chris-allan
Copy link
Member Author

Example when used on a small table:

In [1]: sr = client.getSession().sharedResources()
   ...: a = sr.openTable(omero.model.OriginalFileI(24965, False))

In [2]: a.slice([0], [3, 0], {"omero.tables.include_row_numbers": "true"})
Out[2]:
object #0 (::omero::grid::Data)
{
    lastModification = 1718797774346
    rowNumbers =
    {
        [0] = 3
        [1] = 0
    }
    columns =
    {
        [0] = object #1 (::omero::grid::StringColumn)
        {
            name = ImageName
            description =
            size = 53
            values =
            {
                [0] = siControl_N20_Cep215_I_20110411_Mon-1509_0_SIR_PRJ.dv
                [1] = Centrin_PCNT_Cep215_20110506_Fri-1608_0_SIR_PRJ.dv
            }
        }
    }
}

In [3]: a.slice([0], [3, 0], {"omero.tables.include_row_numbers": "false"})
Out[3]:
object #0 (::omero::grid::Data)
{
    lastModification = 1718797774346
    rowNumbers =
    {
    }
    columns =
    {
        [0] = object #1 (::omero::grid::StringColumn)
        {
            name = ImageName
            description =
            size = 53
            values =
            {
                [0] = siControl_N20_Cep215_I_20110411_Mon-1509_0_SIR_PRJ.dv
                [1] = Centrin_PCNT_Cep215_20110506_Fri-1608_0_SIR_PRJ.dv
            }
        }
    }
}

If we're happy with the implementation I'll make separate PRs to add integration tests like we have for the bitmask query and update the main OMERO.tables documentation detailing the feature.

Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

👍

I'm surprised that the row numbers are longer than other columns except perhaps bools 😏 but I can definitely see how they would effectively double the overhead.

@chris-allan
Copy link
Member Author

👍

I'm surprised that the row numbers are longer than other columns except perhaps bools 😏 but I can definitely see how they would effectively double the overhead.

Also true for short string columns. When it comes to memory usage in Python in particular, also true where the same numbers or strings repeat. These are both common in a lot of the data analysis outputs we're exposed to.

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

Code looks good and functional testing against merge-ci server worked as described (default behaviour unchanged etc).

chris-allan added a commit to chris-allan/openmicroscopy that referenced this pull request Jul 8, 2024
@chris-allan
Copy link
Member Author

Integration test added in ome/openmicroscopy#6396.

chris-allan added a commit to chris-allan/ome-documentation that referenced this pull request Jul 8, 2024
@chris-allan
Copy link
Member Author

Documentation added in ome/omero-documentation#2441.

snoopycrimecop added a commit to snoopycrimecop/openmicroscopy that referenced this pull request Jul 10, 2024
snoopycrimecop added a commit to snoopycrimecop/openmicroscopy that referenced this pull request Jul 10, 2024
Repository: ome/openmicroscopy
Excluded PRs:
  - PR 6379 jburel 'TMP: build david's s3 Zarr branch' (exclude comment)
  - PR 6319 dominikl 'Add integration test for joining session' (exclude comment)
  - PR 6275 joshmoore 'Add OMERO5.4__1 for omero-model#71' (exclude comment)
  - PR 6212 jburel 'add test to import images' (exclude comment)
  - PR 6086 manics 'Alternative JSON configuration system for OMERO.web' (label: exclude)
Already up to date.

Merged PRs:
  - PR 6309 jburel 'add try block'
  - PR 6353 jburel 'remove dependencies'
  - PR 6376 snoopycrimecop 'update dependencies'
  - PR 6385 jburel 'remove test using deprecated method'
  - PR 6388 sbesson 'Upgrade Ivy to 2.5.2'
  - PR 6389 chris-allan 'Handle archive status being populated (See ome/omero-web#555)'
  - PR 6393 sbesson 'Add documentation for the omero.logging properties'
  - PR 6395 sbesson 'Update IceGrid templates to make the OMERO.tables module configurable'
  - PR 6396 chris-allan 'Add a test case for ome/omero-py#418'

Generated by OMERO-push#112 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-push/112/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 10, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 10, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'

Generated by OMERO-docs#113 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/113/)
snoopycrimecop added a commit to snoopycrimecop/openmicroscopy that referenced this pull request Jul 11, 2024
snoopycrimecop added a commit to snoopycrimecop/openmicroscopy that referenced this pull request Jul 11, 2024
Repository: ome/openmicroscopy
Excluded PRs:
  - PR 6379 jburel 'TMP: build david's s3 Zarr branch' (exclude comment)
  - PR 6319 dominikl 'Add integration test for joining session' (exclude comment)
  - PR 6275 joshmoore 'Add OMERO5.4__1 for omero-model#71' (exclude comment)
  - PR 6212 jburel 'add test to import images' (exclude comment)
  - PR 6086 manics 'Alternative JSON configuration system for OMERO.web' (label: exclude)
Already up to date.

Merged PRs:
  - PR 6309 jburel 'add try block'
  - PR 6353 jburel 'remove dependencies'
  - PR 6376 snoopycrimecop 'update dependencies'
  - PR 6385 jburel 'remove test using deprecated method'
  - PR 6388 sbesson 'Upgrade Ivy to 2.5.2'
  - PR 6389 chris-allan 'Handle archive status being populated (See ome/omero-web#555)'
  - PR 6393 sbesson 'Add documentation for the omero.logging properties'
  - PR 6395 sbesson 'Update IceGrid templates to make the OMERO.tables module configurable'
  - PR 6396 chris-allan 'Add a test case for ome/omero-py#418'

Generated by OMERO-push#113 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-push/113/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 11, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 11, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2443 snoopycrimecop 'Changes from upstream repositories:  openmicroscopy' (status: failure)
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'
  - PR 2442 jburel 'Debian10'

Generated by OMERO-docs#114 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/114/)
snoopycrimecop added a commit to snoopycrimecop/openmicroscopy that referenced this pull request Jul 12, 2024
snoopycrimecop added a commit to snoopycrimecop/openmicroscopy that referenced this pull request Jul 12, 2024
Repository: ome/openmicroscopy
Excluded PRs:
  - PR 6379 jburel 'TMP: build david's s3 Zarr branch' (exclude comment)
  - PR 6319 dominikl 'Add integration test for joining session' (exclude comment)
  - PR 6275 joshmoore 'Add OMERO5.4__1 for omero-model#71' (exclude comment)
  - PR 6212 jburel 'add test to import images' (exclude comment)
  - PR 6086 manics 'Alternative JSON configuration system for OMERO.web' (label: exclude)
Already up to date.

Merged PRs:
  - PR 6309 jburel 'add try block'
  - PR 6353 jburel 'remove dependencies'
  - PR 6376 snoopycrimecop 'update dependencies'
  - PR 6385 jburel 'remove test using deprecated method'
  - PR 6388 sbesson 'Upgrade Ivy to 2.5.2'
  - PR 6389 chris-allan 'Handle archive status being populated (See ome/omero-web#555)'
  - PR 6393 sbesson 'Add documentation for the omero.logging properties'
  - PR 6395 sbesson 'Update IceGrid templates to make the OMERO.tables module configurable'
  - PR 6396 chris-allan 'Add a test case for ome/omero-py#418'

Generated by OMERO-push#114 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-push/114/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 12, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 12, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2443 snoopycrimecop 'Changes from upstream repositories:  openmicroscopy' (status: failure)
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'
  - PR 2442 jburel 'Debian10'

Generated by OMERO-docs#115 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/115/)
@jburel jburel merged commit 2e78eea into ome:master Jul 12, 2024
13 checks passed
jburel added a commit to ome/openmicroscopy that referenced this pull request Jul 12, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 13, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 13, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'
  - PR 2444 snoopycrimecop 'Changes from upstream repositories:  openmicroscopy'

Generated by OMERO-docs#116 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/116/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 14, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 14, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'

Generated by OMERO-docs#117 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/117/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 15, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 15, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'

Generated by OMERO-docs#118 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/118/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 16, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 16, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'

Generated by OMERO-docs#119 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/119/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 17, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 17, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'

Generated by OMERO-docs#120 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/120/)
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 18, 2024
snoopycrimecop added a commit to snoopycrimecop/ome-documentation that referenced this pull request Jul 18, 2024
Repository: ome/ome-documentation
Excluded PRs:
  - PR 2432 ehrenfeu 'Some suggestions on the OMERO.movie page' (user: ehrenfeu)
  - PR 2393 bernt-matthias 'fix option capitalization' (user: bernt-matthias)
  - PR 2357 fmeyenhofer 'Add a OMERO.table visualisation example' (user: fmeyenhofer)
  - PR 2347 jburel 'pin version to match rtd ones' (exclude comment)
  - PR 2298 pwalczysko 'test' (exclude comment)
Already up to date.

Merged PRs:
  - PR 2291 pwalczysko 'Add ansible and docker install doc -  former "install.rst"'
  - PR 2353 jburel 'Mencoder'
  - PR 2355 will-moore 'Update python Tables code to use 'Image' column'
  - PR 2424 DavidStirling 'Clarify CSRF token usage'
  - PR 2430 jburel 'exclude trac'
  - PR 2441 chris-allan 'Add documentation for ome/omero-py#418'

Generated by OMERO-docs#121 (https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-docs/121/)
jburel added a commit to ome/omero-documentation that referenced this pull request Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants