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

Add test for image names when resolutions are not flattened #4114

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

melissalinkert
Copy link
Member

In most cases, the Image names stored in a MetadataStore will not depend on hasFlattenedResolutions(). For some pyramid formats (e.g. SVS, CZI), the Image names are noticeably different.

This updates the config file generation to add an Alternate_Name representing the unflattened Image name, if the unflattened Image name is different from the flattened Image name for a particular series/resolution. The new testUnflattenedImageNames checks this value, and errors if the name provided by an unflattened reader does not match either configured name.

I would expect this to cause test failures for SVS, CZI, and potentially others. I am happy for this to be excluded once at least test run has completed (so we know the scope of config changes required).

…tened

Some readers report noticeably different image names based on the resolution
flattening setting. This allows an alternate image name to be configured,
so that both sets of image names can be checked for regressions.
@sbesson
Copy link
Member

sbesson commented Nov 13, 2023

On the positive side, it looks like tests have been failing if and only if the format is multi-resolution which was the expectation.

@sbesson sbesson added this to the 7.2.0 milestone Dec 18, 2023
@sbesson
Copy link
Member

sbesson commented Dec 18, 2023

As discussed today with @melissalinkert and @joshmoore, I agree the latest state of this PR puts us in a nice compromise between:

  • allowing the non-regression test to check image names when using the sub-resolution API - which is a prerequisite to add coverage for issues of the like of CZI: Label and macro image incorrectly labelled #4103
  • and making minimal changes to the existing test-suite infrastructure and the set of existing configuration samples.

Next step here is to update the configuration files for all pyramidal file formats and re-include this PR in the nightly CI builds

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.

Together with the associated configuration changes for all multi-resolution formats, the non-regression tests has been passing over several execution of the nightly CI builds

Copy link
Member

@dgault dgault left a comment

Choose a reason for hiding this comment

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

The approach taken here seems sensible to me and the new test looks good. With all of the config updates the tests have been passing without any issues.

@dgault dgault merged commit ff12362 into ome:develop Jan 10, 2024
17 checks passed
@melissalinkert melissalinkert deleted the image-name-test branch September 6, 2024 19:00
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.

3 participants