Skip to content

Commit 3b90f1c

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 e40924f commit 3b90f1c

3 files changed

Lines changed: 41 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: 22 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;
@@ -50,7 +51,7 @@ pub(crate) struct CodecPipelineImpl {
5051
/// The writable handle -- `None` when zarr-python opened the store read-only. Same object
5152
/// as `readable_store`; the read-only case simply never keeps a writable view of it.
5253
pub(crate) writable_store: Option<ReadableWritableListableStorage>,
53-
pub(crate) codec_chain: Arc<CodecChain>,
54+
pub(crate) codec_chain: Arc<CodecChainBound>,
5455
pub(crate) codec_options: CodecOptions,
5556
pub(crate) chunk_concurrent_minimum: usize,
5657
pub(crate) chunk_concurrent_maximum: usize,
@@ -63,7 +64,7 @@ impl CodecPipelineImpl {
6364
fn retrieve_chunk_bytes<'a>(
6465
&self,
6566
item: &ChunkItem,
66-
codec_chain: &CodecChain,
67+
codec_chain: &CodecChainBound,
6768
codec_options: &CodecOptions,
6869
) -> PyResult<ArrayBytes<'a>> {
6970
let value_encoded = self
@@ -73,13 +74,7 @@ impl CodecPipelineImpl {
7374
let value_decoded = if let Some(value_encoded) = value_encoded {
7475
let value_encoded: Vec<u8> = value_encoded.into(); // zero-copy in this case
7576
codec_chain
76-
.decode(
77-
value_encoded.into(),
78-
&item.shape,
79-
&self.data_type,
80-
&self.fill_value,
81-
codec_options,
82-
)
77+
.decode(value_encoded.into(), &item.shape, codec_options)
8378
.map_codec_err()?
8479
} else {
8580
ArrayBytes::new_fill_value(&self.data_type, item.num_elements, &self.fill_value)
@@ -98,7 +93,7 @@ impl CodecPipelineImpl {
9893
fn store_chunk_bytes(
9994
&self,
10095
item: &ChunkItem,
101-
codec_chain: &CodecChain,
96+
codec_chain: &CodecChainBound,
10297
value_decoded: ArrayBytes,
10398
codec_options: &CodecOptions,
10499
) -> PyResult<()> {
@@ -112,13 +107,7 @@ impl CodecPipelineImpl {
112107
store.erase(&item.key).map_py_err::<PyRuntimeError>()
113108
} else {
114109
let value_encoded = codec_chain
115-
.encode(
116-
value_decoded,
117-
&item.shape,
118-
&self.data_type,
119-
&self.fill_value,
120-
codec_options,
121-
)
110+
.encode(value_decoded, &item.shape, codec_options)
122111
.map(Cow::into_owned)
123112
.map_codec_err()?;
124113

@@ -132,7 +121,7 @@ impl CodecPipelineImpl {
132121
fn store_chunk_subset_bytes(
133122
&self,
134123
item: &ChunkItem,
135-
codec_chain: &CodecChain,
124+
codec_chain: &CodecChainBound,
136125
chunk_subset_bytes: ArrayBytes,
137126
codec_options: &CodecOptions,
138127
) -> PyResult<()> {
@@ -257,8 +246,10 @@ impl CodecPipelineImpl {
257246
}
258247
ArrayMetadata::V3(v3) => Cow::Borrowed(v3),
259248
};
249+
// Parsed before binding, so an array with bad codecs and a bad fill value still
250+
// reports the codecs.
260251
let codec_chain =
261-
Arc::new(CodecChain::from_metadata(&metadata_v3.codecs).map_py_err::<PyTypeError>()?);
252+
CodecChain::from_metadata(&metadata_v3.codecs).map_py_err::<PyTypeError>()?;
262253
let codec_options = CodecOptions::default().with_validate_checksums(validate_checksums);
263254

264255
let chunk_concurrent_minimum =
@@ -288,6 +279,10 @@ impl CodecPipelineImpl {
288279
})
289280
.map_py_err::<PyTypeError>()?;
290281

282+
let codec_chain = codec_chain
283+
.with_context(data_type.clone(), fill_value.clone())
284+
.map_py_err::<PyTypeError>()?;
285+
291286
Ok(Self {
292287
readable_store,
293288
codec_chain,
@@ -328,18 +323,15 @@ impl CodecPipelineImpl {
328323
if !partial_chunk_items.is_empty() {
329324
let key_decoder_pairs =
330325
iter_concurrent_limit!(chunk_concurrent_limit, partial_chunk_items, map, |item| {
331-
let storage_handle = Arc::new(StorageHandle::new(self.readable_store.clone()));
332-
let input_handle = StoragePartialDecoder::new(storage_handle, item.key.clone());
326+
// The (storage, key) tuple IS the store-backed `BytesPartialDecoderTraits`.
327+
let input_handle: Arc<dyn BytesPartialDecoderTraits> = Arc::new((
328+
StorageHandle::new(self.readable_store.clone()),
329+
item.key.clone(),
330+
));
333331
let partial_decoder = self
334332
.codec_chain
335333
.clone()
336-
.partial_decoder(
337-
Arc::new(input_handle),
338-
&item.shape,
339-
&self.data_type,
340-
&self.fill_value,
341-
&codec_options,
342-
)
334+
.partial_decoder(input_handle, &item.shape, &codec_options)
343335
.map_codec_err()?;
344336
Ok((item.key.clone(), partial_decoder))
345337
})
@@ -382,8 +374,6 @@ impl CodecPipelineImpl {
382374
self.codec_chain.decode_into(
383375
Cow::Owned(chunk_encoded),
384376
&item.shape,
385-
&self.data_type,
386-
&self.fill_value,
387377
target,
388378
&codec_options,
389379
)

0 commit comments

Comments
 (0)