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

Make Manifests' in-memory reference structure pluggable #246

Open
TomNicholas opened this issue Oct 3, 2024 · 0 comments
Open

Make Manifests' in-memory reference structure pluggable #246

TomNicholas opened this issue Oct 3, 2024 · 0 comments

Comments

@TomNicholas
Copy link
Member

Currently the ChunkManifest is hardcoded to use numpy arrays underneath to store the paths/offsets/byte ranges. However there are a few cases where we might want to use another format:

We could definitely imagine making this pluggable. The main question I have is whether the other manifest implementations should implement their own ChunkManifest class (e.g. SparseChunkManifest), i.e. ManifestArray becomes a Generic in the type of the .manifest attribute; or use virtualizarr's ChunkManifest class but wrap a different array type, i.e. ChunkManifest becomes a Generic in the type of the .paths/.offset/.lengths attributes.

Right now the latter should be pretty straightforward, but the former would require some refactoring because the ChunkManifest abstraction is leaky in that the implementation of concatenate for ManifestArrays accesses private internals of the wrapped ChunkManifest (i.e. the wrapped numpy arrays).

A related consideration is providing some kind of interface for iterating over all the references in the Manifest that doesn't make assumptions about how the references are actually stored under the hood. That's currently another place where the abstraction is a bit leaky, e.g.

renamed_paths = vectorized_rename_fn(self._paths)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant