Skip to content

Fix issue #727#800

Open
frothga wants to merge 5 commits intojamesmudd:masterfrom
frothga:master
Open

Fix issue #727#800
frothga wants to merge 5 commits intojamesmudd:masterfrom
frothga:master

Conversation

@frothga
Copy link
Copy Markdown

@frothga frothga commented Mar 8, 2026

Fix scaling of chunk coordinates in ChunkedDatasetBase.findOverlappingChunks()

Copy link
Copy Markdown
Owner

@jamesmudd jamesmudd left a comment

Choose a reason for hiding this comment

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

This fix looks reasonable but would it be possible to add tests that would have caught this?

@frothga
Copy link
Copy Markdown
Author

frothga commented Mar 10, 2026

I'll take a look at your test system. This bug should occur on any read outside the first chunk. Such reads will return no chunk, or in very rare cases a completely wrong chunk.

@frothga
Copy link
Copy Markdown
Author

frothga commented Mar 10, 2026

I briefly looked over the tests that involve chunks. At least in the case of ChunkSlicingTest, the bug is not revealed because the chunk size in the selected datasets is 1x1x1. For an effective test, it would be necessary to create a file with some larger, perhaps odd-sized, chunk. The existing test code is probably sufficient if chunked_v4_datasets.hdf5 were reworked a little.

@jamesmudd
Copy link
Copy Markdown
Owner

Thanks for investigating I think the existing test files should have suitable datasets already take a look at https://github.com/jamesmudd/jhdf/blob/master/jhdf/src/test/resources/scripts/chunked_v4_datasets.py

@frothga
Copy link
Copy Markdown
Author

frothga commented Mar 11, 2026

I'm a little confused. I think what you mean is that chunked_v4_datasets.py should be modified so that the large_int16 datasets have a chunk size larger than 1x1x1.

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.

2 participants