Skip to content

Commit

Permalink
Refactor partial decoder API (#76)
Browse files Browse the repository at this point in the history
  • Loading branch information
LDeakin authored Sep 19, 2024
1 parent 26b90dd commit cfb65d4
Show file tree
Hide file tree
Showing 54 changed files with 638 additions and 618 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed
- **Breaking**: Change `Contiguous[Linearised]Indices` iterators to return indices only
- **Breaking**: Remove lifetime constraints in partial decoder API by utilising `Arc`'d codecs
- **Breaking**: `ShardingCodec::new` `CodecChain` parameters now must be in an `Arc`

### Fixed
- Fixed an unnecessary copy in `Array::[async_]retrieve_chunk_if_exists_opt`
Expand Down
10 changes: 6 additions & 4 deletions zarrs/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ pub struct Array<TStorage: ?Sized> {
/// Provides an element value to use for uninitialised portions of the Zarr array. It encodes the underlying data type.
fill_value: FillValue,
/// Specifies a list of codecs to be used for encoding and decoding chunks.
codecs: CodecChain,
codecs: Arc<CodecChain>,
// /// Optional user defined attributes.
// attributes: serde_json::Map<String, serde_json::Value>,
/// An optional list of storage transformers.
Expand Down Expand Up @@ -352,8 +352,10 @@ impl<TStorage: ?Sized> Array<TStorage> {
let fill_value = data_type
.fill_value_from_metadata(&metadata_v3.fill_value)
.map_err(ArrayCreateError::InvalidFillValueMetadata)?;
let codecs = CodecChain::from_metadata(&metadata_v3.codecs)
.map_err(ArrayCreateError::CodecsCreateError)?;
let codecs = Arc::new(
CodecChain::from_metadata(&metadata_v3.codecs)
.map_err(ArrayCreateError::CodecsCreateError)?,
);
let storage_transformers =
StorageTransformerChain::from_metadata(&metadata_v3.storage_transformers)
.map_err(ArrayCreateError::StorageTransformersCreateError)?;
Expand Down Expand Up @@ -432,7 +434,7 @@ impl<TStorage: ?Sized> Array<TStorage> {

/// Get the codecs.
#[must_use]
pub const fn codecs(&self) -> &CodecChain {
pub fn codecs(&self) -> &CodecChain {
&self.codecs
}

Expand Down
21 changes: 12 additions & 9 deletions zarrs/src/array/array_async_readable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,10 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits + 'static> Array<TStorage> {

/// Async variant of [`partial_decoder`](Array::partial_decoder).
#[allow(clippy::missing_errors_doc, clippy::missing_panics_doc)]
pub async fn async_partial_decoder<'a>(
&'a self,
pub async fn async_partial_decoder(
&self,
chunk_indices: &[u64],
) -> Result<Arc<dyn AsyncArrayPartialDecoderTraits + 'a>, ArrayError> {
) -> Result<Arc<dyn AsyncArrayPartialDecoderTraits>, ArrayError> {
self.async_partial_decoder_opt(chunk_indices, &CodecOptions::default())
.await
}
Expand Down Expand Up @@ -768,7 +768,8 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits + 'static> Array<TStorage> {
storage_transformer,
self.chunk_key(chunk_indices),
));
self.codecs()
self.codecs
.clone()
.async_partial_decoder(input_handle, &chunk_representation, options)
.await?
.partial_decode_opt(&[chunk_subset.clone()], options)
Expand Down Expand Up @@ -820,7 +821,8 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits + 'static> Array<TStorage> {
));

Ok(self
.codecs()
.codecs
.clone()
.async_partial_decoder(input_handle, &chunk_representation, options)
.await?
.partial_decode_into(chunk_subset, output, output_shape, output_subset, options)
Expand Down Expand Up @@ -860,11 +862,11 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits + 'static> Array<TStorage> {

/// Async variant of [`partial_decoder_opt`](Array::partial_decoder_opt).
#[allow(clippy::missing_errors_doc)]
pub async fn async_partial_decoder_opt<'a>(
&'a self,
pub async fn async_partial_decoder_opt(
&self,
chunk_indices: &[u64],
options: &CodecOptions,
) -> Result<Arc<dyn AsyncArrayPartialDecoderTraits + 'a>, ArrayError> {
) -> Result<Arc<dyn AsyncArrayPartialDecoderTraits>, ArrayError> {
let storage_handle = Arc::new(StorageHandle::new(self.storage.clone()));
let storage_transformer = self
.storage_transformers()
Expand All @@ -875,7 +877,8 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits + 'static> Array<TStorage> {
));
let chunk_representation = self.chunk_array_representation(chunk_indices)?;
Ok(self
.codecs()
.codecs
.clone()
.async_partial_decoder(input_handle, &chunk_representation, options)
.await?)
}
Expand Down
4 changes: 2 additions & 2 deletions zarrs/src/array/array_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,11 @@ impl ArrayBuilder {
chunk_grid: self.chunk_grid.clone(),
chunk_key_encoding: self.chunk_key_encoding.clone(),
fill_value: self.fill_value.clone(),
codecs: CodecChain::new(
codecs: Arc::new(CodecChain::new(
self.array_to_array_codecs.clone(),
self.array_to_bytes_codec.clone(),
self.bytes_to_bytes_codecs.clone(),
),
)),
storage_transformers: self.storage_transformers.clone(),
// attributes: self.attributes.clone(),
dimension_names: self.dimension_names.clone(),
Expand Down
21 changes: 12 additions & 9 deletions zarrs/src/array/array_sync_readable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,10 +407,10 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> Array<TStorage> {
///
/// # Errors
/// Returns an [`ArrayError`] if initialisation of the partial decoder fails.
pub fn partial_decoder<'a>(
&'a self,
pub fn partial_decoder(
&self,
chunk_indices: &[u64],
) -> Result<Arc<dyn ArrayPartialDecoderTraits + 'a>, ArrayError> {
) -> Result<Arc<dyn ArrayPartialDecoderTraits>, ArrayError> {
self.partial_decoder_opt(chunk_indices, &CodecOptions::default())
}

Expand Down Expand Up @@ -830,7 +830,8 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> Array<TStorage> {
self.chunk_key(chunk_indices),
));

self.codecs()
self.codecs
.clone()
.partial_decoder(input_handle, &chunk_representation, options)?
.partial_decode_opt(&[chunk_subset.clone()], options)?
.remove(0)
Expand Down Expand Up @@ -873,7 +874,8 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> Array<TStorage> {
));

Ok(self
.codecs()
.codecs
.clone()
.partial_decoder(input_handle, &chunk_representation, options)?
.partial_decode_into(chunk_subset, output, output_shape, output_subset, options)?)
}
Expand Down Expand Up @@ -909,11 +911,11 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> Array<TStorage> {

/// Explicit options version of [`partial_decoder`](Array::partial_decoder).
#[allow(clippy::missing_errors_doc)]
pub fn partial_decoder_opt<'a>(
&'a self,
pub fn partial_decoder_opt(
&self,
chunk_indices: &[u64],
options: &CodecOptions,
) -> Result<Arc<dyn ArrayPartialDecoderTraits + 'a>, ArrayError> {
) -> Result<Arc<dyn ArrayPartialDecoderTraits>, ArrayError> {
let storage_handle = Arc::new(StorageHandle::new(self.storage.clone()));
let storage_transformer = self
.storage_transformers()
Expand All @@ -924,7 +926,8 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> Array<TStorage> {
));
let chunk_representation = self.chunk_array_representation(chunk_indices)?;
Ok(self
.codecs()
.codecs
.clone()
.partial_decoder(input_handle, &chunk_representation, options)?)
}
}
Loading

0 comments on commit cfb65d4

Please sign in to comment.