Skip to content

Commit 824b5de

Browse files
committed
chore: migrate to zarrs 0.24
A dependency bump and the API migration it forces, and nothing else. Kept on its own because a dependency migration reviewed together with a behaviour change means neither can be reverted alone. All of it is one upstream idea -- the #411 refactor -- which is that a codec chain is BOUND to a data type and fill value once, where it is built, instead of being handed both at every call. `CodecChain` is now unbound; `decode`, `encode`, `decode_into`, `partial_decoder` and `recommended_concurrency` live on `CodecChainBound`, reached by `with_context`. So the chain is bound in the constructor and five call sites stop passing a pair they no longer need. `zarrs::array::StoragePartialDecoder` is also gone, and nothing named replaced it: the (storage, key) TUPLE is the store-backed BytesPartialDecoderTraits implementation now. No behaviour change, with one exception worth naming rather than hiding: a codec chain that cannot BIND to its data type now fails in the constructor as a TypeError, where before it surfaced at the first read as a RuntimeError. Binding has to happen somewhere, and the constructor is the only place it can. The codec metadata is still PARSED where it was, so an array with both bad codecs and a bad fill value still reports the codecs. The dependency is a git rev rather than a version, because 0.24 is not released yet, and it carries a [patch.crates-io] block for zarrs_storage. That patch is load-bearing, not cosmetic: zarrs_opendal and zarrs_object_store track RELEASED zarrs, so they pull zarrs_storage from crates.io while zarrs comes from git -- two copies of one crate, two distinct AsyncReadableStorageTraits, and trait bounds that cannot be satisfied. Patching that single crate collapses the graph; every other zarrs crate already resolves through the git checkout, verified from a clean lockfile. Both the pin and the patch are commented with exactly what to do on release day, and nothing in src/ changes then.
1 parent 3345fd8 commit 824b5de

3 files changed

Lines changed: 39 additions & 34 deletions

File tree

Cargo.toml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ crate-type = ["cdylib", "rlib"]
1010

1111
[dependencies]
1212
pyo3 = { version = "0.27.1", features = ["abi3-py311"] }
13-
zarrs = { version = "0.23.6", features = ["async", "zlib", "pcodec", "bz2"] }
13+
# ON THE 0.24 RELEASE: replace with `zarrs = { version = "0.24", features = [...] }` AND
14+
# delete the [patch.crates-io] block below, in the SAME commit -- see the note there for why
15+
# doing only one of the two builds silently against the wrong `zarrs_storage`. src/ already
16+
# compiles against the 0.24 API.
17+
zarrs = { git = "https://github.com/zarrs/zarrs", rev = "c17fe374b1fa7df8373b6c6f6eb3f1d33c3a3bd7", features = ["async", "zlib", "pcodec", "bz2"] }
1418
rayon_iter_concurrent_limit = "0.2.0"
1519
rayon = "1.10.0"
1620
# fix for https://stackoverflow.com/questions/76593417/package-openssl-was-not-found-in-the-pkg-config-search-path
@@ -29,3 +33,16 @@ zarrs_object_store = "0.5.0" # object_store 0.12
2933

3034
[profile.release]
3135
lto = true
36+
37+
# ON THE 0.24 RELEASE: delete this, in the same commit that drops the git rev above.
38+
#
39+
# `zarrs_opendal` and `zarrs_object_store` do not depend on `zarrs` at all -- only on
40+
# `zarrs_storage`, from crates.io. While `zarrs` comes from git that is a SECOND copy of
41+
# `zarrs_storage`, so there are two `AsyncReadableStorageTraits` and the bounds on the async
42+
# stores cannot be satisfied. Patching the one crate to the same rev collapses the graph.
43+
#
44+
# Drop the git rev above WITHOUT deleting this and the build is silently wrong: released
45+
# `zarrs 0.24` compiled against an unpublished `zarrs_storage`. Nothing warns, because the
46+
# git copy carries the same version number as the published one with different contents.
47+
[patch.crates-io]
48+
zarrs_storage = { git = "https://github.com/zarrs/zarrs", rev = "c17fe374b1fa7df8373b6c6f6eb3f1d33c3a3bd7" }

src/concurrency.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ impl ChunkConcurrentLimitAndCodecOptions for Vec<ChunkItem> {
2525

2626
let codec_concurrency = codec_pipeline_impl
2727
.codec_chain
28-
.recommended_concurrency(&item.shape, &codec_pipeline_impl.data_type)
28+
.recommended_concurrency(&item.shape)
2929
.map_codec_err()?;
3030

3131
let min_concurrent_chunks =

src/lib.rs

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ use rayon::iter::{IntoParallelIterator, ParallelIterator};
1818
use rayon_iter_concurrent_limit::iter_concurrent_limit;
1919
use unsafe_cell_slice::UnsafeCellSlice;
2020
use utils::is_whole_chunk;
21+
use zarrs::array::codec::api::BytesPartialDecoderTraits;
2122
use zarrs::array::{
2223
ArrayBytes, ArrayBytesDecodeIntoTarget, ArrayBytesFixedDisjointView, ArrayMetadata,
23-
ArrayPartialDecoderTraits, ArrayToBytesCodecTraits, CodecChain, CodecOptions, DataType,
24-
FillValue, StoragePartialDecoder, copy_fill_value_into, update_array_bytes,
24+
ArrayPartialDecoderTraits, ArrayToBytesCodecTraits, CodecChain, CodecChainBound, CodecOptions,
25+
DataType, FillValue, copy_fill_value_into, update_array_bytes,
2526
};
2627
use zarrs::config::global_config;
2728
use zarrs::convert::array_metadata_v2_to_v3;
@@ -45,7 +46,7 @@ use crate::utils::{PyCodecErrExt, PyErrExt as _};
4546
#[pyclass]
4647
pub struct CodecPipelineImpl {
4748
pub(crate) store: ReadableWritableListableStorage,
48-
pub(crate) codec_chain: Arc<CodecChain>,
49+
pub(crate) codec_chain: Arc<CodecChainBound>,
4950
pub(crate) codec_options: CodecOptions,
5051
pub(crate) chunk_concurrent_minimum: usize,
5152
pub(crate) chunk_concurrent_maximum: usize,
@@ -58,20 +59,14 @@ impl CodecPipelineImpl {
5859
fn retrieve_chunk_bytes<'a>(
5960
&self,
6061
item: &ChunkItem,
61-
codec_chain: &CodecChain,
62+
codec_chain: &CodecChainBound,
6263
codec_options: &CodecOptions,
6364
) -> PyResult<ArrayBytes<'a>> {
6465
let value_encoded = self.store.get(&item.key).map_py_err::<PyRuntimeError>()?;
6566
let value_decoded = if let Some(value_encoded) = value_encoded {
6667
let value_encoded: Vec<u8> = value_encoded.into(); // zero-copy in this case
6768
codec_chain
68-
.decode(
69-
value_encoded.into(),
70-
&item.shape,
71-
&self.data_type,
72-
&self.fill_value,
73-
codec_options,
74-
)
69+
.decode(value_encoded.into(), &item.shape, codec_options)
7570
.map_codec_err()?
7671
} else {
7772
ArrayBytes::new_fill_value(&self.data_type, item.num_elements, &self.fill_value)
@@ -83,7 +78,7 @@ impl CodecPipelineImpl {
8378
fn store_chunk_bytes(
8479
&self,
8580
item: &ChunkItem,
86-
codec_chain: &CodecChain,
81+
codec_chain: &CodecChainBound,
8782
value_decoded: ArrayBytes,
8883
codec_options: &CodecOptions,
8984
) -> PyResult<()> {
@@ -95,13 +90,7 @@ impl CodecPipelineImpl {
9590
self.store.erase(&item.key).map_py_err::<PyRuntimeError>()
9691
} else {
9792
let value_encoded = codec_chain
98-
.encode(
99-
value_decoded,
100-
&item.shape,
101-
&self.data_type,
102-
&self.fill_value,
103-
codec_options,
104-
)
93+
.encode(value_decoded, &item.shape, codec_options)
10594
.map(Cow::into_owned)
10695
.map_codec_err()?;
10796

@@ -115,7 +104,7 @@ impl CodecPipelineImpl {
115104
fn store_chunk_subset_bytes(
116105
&self,
117106
item: &ChunkItem,
118-
codec_chain: &CodecChain,
107+
codec_chain: &CodecChainBound,
119108
chunk_subset_bytes: ArrayBytes,
120109
codec_options: &CodecOptions,
121110
) -> PyResult<()> {
@@ -240,8 +229,10 @@ impl CodecPipelineImpl {
240229
}
241230
ArrayMetadata::V3(v3) => Cow::Borrowed(v3),
242231
};
232+
// Parsed before binding, so an array with bad codecs and a bad fill value still
233+
// reports the codecs.
243234
let codec_chain =
244-
Arc::new(CodecChain::from_metadata(&metadata_v3.codecs).map_py_err::<PyTypeError>()?);
235+
CodecChain::from_metadata(&metadata_v3.codecs).map_py_err::<PyTypeError>()?;
245236
let codec_options = CodecOptions::default().with_validate_checksums(validate_checksums);
246237

247238
let chunk_concurrent_minimum =
@@ -271,6 +262,10 @@ impl CodecPipelineImpl {
271262
})
272263
.map_py_err::<PyTypeError>()?;
273264

265+
let codec_chain = codec_chain
266+
.with_context(data_type.clone(), fill_value.clone())
267+
.map_py_err::<PyTypeError>()?;
268+
274269
Ok(Self {
275270
store,
276271
codec_chain,
@@ -310,18 +305,13 @@ impl CodecPipelineImpl {
310305
if !partial_chunk_items.is_empty() {
311306
let key_decoder_pairs =
312307
iter_concurrent_limit!(chunk_concurrent_limit, partial_chunk_items, map, |item| {
313-
let storage_handle = Arc::new(StorageHandle::new(self.store.clone()));
314-
let input_handle = StoragePartialDecoder::new(storage_handle, item.key.clone());
308+
// The (storage, key) tuple IS the store-backed `BytesPartialDecoderTraits`.
309+
let input_handle: Arc<dyn BytesPartialDecoderTraits> =
310+
Arc::new((StorageHandle::new(self.store.clone()), item.key.clone()));
315311
let partial_decoder = self
316312
.codec_chain
317313
.clone()
318-
.partial_decoder(
319-
Arc::new(input_handle),
320-
&item.shape,
321-
&self.data_type,
322-
&self.fill_value,
323-
&codec_options,
324-
)
314+
.partial_decoder(input_handle, &item.shape, &codec_options)
325315
.map_codec_err()?;
326316
Ok((item.key.clone(), partial_decoder))
327317
})
@@ -362,8 +352,6 @@ impl CodecPipelineImpl {
362352
self.codec_chain.decode_into(
363353
Cow::Owned(chunk_encoded),
364354
&item.shape,
365-
&self.data_type,
366-
&self.fill_value,
367355
target,
368356
&codec_options,
369357
)

0 commit comments

Comments
 (0)