ZarrReader: HCS validation fixes #24
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Conflicting PR. Removed from build BIOFORMATS-push#51. See the console output for more details.
|
|
Conflicting PR. Removed from build BIOFORMATS-push#72. See the console output for more details.
|
|
It looks like |
|
@dgault: I've pushed a merge commit if wouldn't mind reviewing it. |
Closed
|
There are 4 main things to test with this PR: Validation fixes: This relates to the original issue reported: #22 8 bit data: Without this PR 8 bit data would display as a blank plane. With the PR included 8bit data should display correctly. Any sample file with 8bit pixel data should be suitable for testing, Petr's data at Multi Image, Multi Resolutions: Without this PR multi resolutions on the first image were detected correctly but for subsequent images the resolution levels were not detected. With this PR the resolution levels should be correctly detected for all images. This can be tested using Default DimensionOrder: This was discovered using Petr's test data, in this case the dimension order is not present in the Zarr attributes and the default value was being used, however without this PR there was an inconsistency in the reader for this default order. This has now been corrected to use XYZCT. The file to test would be |
|
Testing 8 bit data issue (nr. 2 from #24 (comment)) Dimension order looks good The image is z=5 as expected. Without this PR This looks good, but maybe for other tests we would profit from having merge-ci back ? Or any suggestions for usage of an OMERO server ? |
|
Tried to test also Ad 3 Multi-image multi resolutions from #24 (comment). With this PR I am getting no errors . Looks good. (edited) |
|
@sbesson happy to discuss the outputs in #24 (comment) in more detail, - is viewing of the image necessary ? |
|
Testing the 8 bit data paragraph on OMERO did fully succeed. Workflow:
Also see The positive the dimensions of the image (t=1, z=5) and the number of the images (just 1) is correct. Edited: after OMERO.server restart and reimport, the image has the correct pixel values. See user-3 https://merge-ci.openmicroscopy.org/web/webclient/?show=image-229601 |
|
@dgault @sbesson edited #24 (comment) - after server restart and reimport, all looks good. |
|
Re: Validation fixes (point 1 of #24 (comment) |
|
In summary, the tests in #24 (comment) are as follows: Validation fixes fails for me with #24 (comment) (tested on idr2-slot3) Seems to me out of 4 tests, 2 full passes, 1 pass with question (OMERO test needed ?) and 1 fail. |
pwalczysko
suggested changes
Apr 24, 2022
|
It seems something has likely broken during the conflict merges or later fixes. I will try and get another commit to resolve the issue and relist for testing when ready |
|
The error reported in #24 (comment) is now fixed |
|
In summary, all looks good. The error is fixed, all files are having either a good |
pwalczysko
approved these changes
Apr 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes a number of fixes for validation issues reported in #22
It also includes some additional fixes for multi res HCS data discovered during testing.
To test you can run the below and verify that no validation issues are reported and that the resolution levels are properly detected:
BF_CP=/bio-formats-build/ZarrReader/target/OMEZarrReader-0.1.3-SNAPSHOT-jar-with-dependencies.jar /bio-formats-build/bioformats/tools/showinf -nopix -noflat -omexml /uod/idr/repos/curated/unsupported/ome-ngff/0.3/idr/idr0094A/7751.zarr/.zattrsFixes #22