BUG: Fix zero-sized slicing and improve test coverage #49
Conversation
This file contains hidden or 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
Fixes an issue where Tensorstore would allow reading a slice of size zero into an empty array buffer, with the result being an array of all zeros.
Adds more verbose printouts to help with debugging and logging
Adds tests to read 2D frames from a contrived 2D+time OME-Zarr test image. Tests include output comparison in MetaImage format to capture metadata and voxel equivalence.
Adds baseline MetaImage comparisons for HTTP tests at various resolution levels. Provides coverage to validate both metadata and voxel agreement with baseline.
thewtex
approved these changes
Aug 11, 2023
dzenanz
approved these changes
Aug 11, 2023
Collaborator
Author
|
I am able to reproduce test failures on my Linux machine. Will investigate. |
Removes metadata validation from HTTP tests. Both metadata validation and image buffer validation are now covered under baseline image comparison after the test body completes.
tbirdso
force-pushed
the
zero-valued-slicing-fix
branch
from
50b55d2 to
3d6984e
Compare
Collaborator
Author
|
Tests are passing after addressing a small test build warning. (had incorrectly marked a test function with Moving forward with merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Addresses issue where time and channel slice sizes were set to zero instead of one. The result of this issue was such that reading from stores with time or channel axes returned correct metadata but all-zero-valued image buffers.
This issue slipped by because I was testing for metadata and voxel array shape correctness, but not the actual content of the images that were produced. I have overhauled testing to cover this case and prevent similar regressions in the future:
t,y,xzarr store with two time frames and validate that each frame has correct metadata and image content.Also adds optional debugging output to
OMEZarrNGFFImageIOto help decipher ITK/tensorstore mismatch issues in the future.