Properly extract chunk keys for arrays with a single chunk #523
Conversation
virtualizarr/manifests/store.py
Outdated
| if key.endswith("c"): | ||
| # Scalar arrays hold the data in the "c" key | ||
| return (0,) |
There was a problem hiding this comment.
I'm not sure I understand this. You're special-casing getting a scalar array by indexing into the underlying manifestarray as if it were 1D with length 1.
But AFAIK the ManifestArray doesn't currently make a special case for that? If anything it looks like scalars might not be implemented??
I also don't understand how your adjustments to the test in this PR cover this if statement added here.
There was a problem hiding this comment.
I'll work on adding a test for this. The existing changes to the tests were unrelated to this and instead properly specified the chunk_key_encoding.
There was a problem hiding this comment.
I decided to just raise a NotImplementedError because this edge case isn't a priority, this PR is slowing progress on TIFF support, and this edge case will be easier to test for after the HDF5 reader uses ManifestStore
|
@TomNicholas Sean and I think #516 is pretty much ready to be merged, but I'd like to get this in first to keep changes isolated between PRs. Do you have any issues with the approach of punting on the scalar array special case? |
|
If it currently doesn't work and you're just formalizing that by raising a |
This PR improves ManifestStore to support ManifestArrays that only contain a single entry in paths, offsets, and lengths. It was originally included in #516 but I've pulled these changes out because they are also needed for the TIFF reader.