From 1f4aebd38635c41df390a19c4ba1c41d5a955ae7 Mon Sep 17 00:00:00 2001 From: Lachlan Deakin Date: Sun, 19 Jan 2025 15:13:16 +1100 Subject: [PATCH 1/3] refactor!: change `ArraySubset::inbounds` to take another subset rather than a shape --- CHANGELOG.md | 6 +++++ zarrs/src/array/array_async_readable.rs | 4 +-- zarrs/src/array/array_sync_readable.rs | 4 +-- .../chunk_cache/array_chunk_cache_ext_sync.rs | 2 +- zarrs/src/array/codec.rs | 6 ++--- zarrs/src/array_subset.rs | 27 +++++++++++++++---- 6 files changed, 36 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2db1ecfd..39a84d5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Add `ArraySubset::inbounds_shape()` (matches the old `ArraySubset::inbounds` behaviour) + +### Changed +- **Breaking**: change `ArraySubset::inbounds` to take another subset rather than a shape + ## [0.19.1] - 2025-01-19 ### Added diff --git a/zarrs/src/array/array_async_readable.rs b/zarrs/src/array/array_async_readable.rs index 79ca0f72..8139b356 100644 --- a/zarrs/src/array/array_async_readable.rs +++ b/zarrs/src/array/array_async_readable.rs @@ -737,7 +737,7 @@ impl Array { options: &CodecOptions, ) -> Result, ArrayError> { let chunk_representation = self.chunk_array_representation(chunk_indices)?; - if !chunk_subset.inbounds(&chunk_representation.shape_u64()) { + if !chunk_subset.inbounds_shape(&chunk_representation.shape_u64()) { return Err(ArrayError::InvalidArraySubset( chunk_subset.clone(), self.shape().to_vec(), @@ -783,7 +783,7 @@ impl Array { options: &CodecOptions, ) -> Result<(), ArrayError> { let chunk_representation = self.chunk_array_representation(chunk_indices)?; - if !chunk_subset.inbounds(&chunk_representation.shape_u64()) { + if !chunk_subset.inbounds_shape(&chunk_representation.shape_u64()) { return Err(ArrayError::InvalidArraySubset( chunk_subset.clone(), self.shape().to_vec(), diff --git a/zarrs/src/array/array_sync_readable.rs b/zarrs/src/array/array_sync_readable.rs index 200e0ba6..a2b8d4b2 100644 --- a/zarrs/src/array/array_sync_readable.rs +++ b/zarrs/src/array/array_sync_readable.rs @@ -794,7 +794,7 @@ impl Array { options: &CodecOptions, ) -> Result, ArrayError> { let chunk_representation = self.chunk_array_representation(chunk_indices)?; - if !chunk_subset.inbounds(&chunk_representation.shape_u64()) { + if !chunk_subset.inbounds_shape(&chunk_representation.shape_u64()) { return Err(ArrayError::InvalidArraySubset( chunk_subset.clone(), self.shape().to_vec(), @@ -837,7 +837,7 @@ impl Array { options: &CodecOptions, ) -> Result<(), ArrayError> { let chunk_representation = self.chunk_array_representation(chunk_indices)?; - if !chunk_subset.inbounds(&chunk_representation.shape_u64()) { + if !chunk_subset.inbounds_shape(&chunk_representation.shape_u64()) { return Err(ArrayError::InvalidArraySubset( chunk_subset.clone(), self.shape().to_vec(), diff --git a/zarrs/src/array/chunk_cache/array_chunk_cache_ext_sync.rs b/zarrs/src/array/chunk_cache/array_chunk_cache_ext_sync.rs index db3f3fee..03cd7036 100644 --- a/zarrs/src/array/chunk_cache/array_chunk_cache_ext_sync.rs +++ b/zarrs/src/array/chunk_cache/array_chunk_cache_ext_sync.rs @@ -229,7 +229,7 @@ impl ArrayChunkCacheExt Result, ArrayError> { let chunk_representation = self.chunk_array_representation(chunk_indices)?; - if !chunk_subset.inbounds(&chunk_representation.shape_u64()) { + if !chunk_subset.inbounds_shape(&chunk_representation.shape_u64()) { return Err(ArrayError::InvalidArraySubset( chunk_subset.clone(), self.shape().to_vec(), diff --git a/zarrs/src/array/codec.rs b/zarrs/src/array/codec.rs index 1850b43c..31e7aa43 100644 --- a/zarrs/src/array/codec.rs +++ b/zarrs/src/array/codec.rs @@ -375,7 +375,7 @@ pub trait ArrayPartialDecoderTraits: Any + Send + Sync { output_subset: &ArraySubset, options: &CodecOptions, ) -> Result<(), CodecError> { - debug_assert!(output_subset.inbounds(output_shape)); + debug_assert!(output_subset.inbounds_shape(output_shape)); debug_assert_eq!(array_subset.num_elements(), output_subset.num_elements()); let decoded_value = self .partial_decode(&[array_subset.clone()], options)? @@ -460,7 +460,7 @@ pub trait AsyncArrayPartialDecoderTraits: Any + Send + Sync { output_subset: &ArraySubset, options: &CodecOptions, ) -> Result<(), CodecError> { - debug_assert!(output_subset.inbounds(output_shape)); + debug_assert!(output_subset.inbounds_shape(output_shape)); debug_assert_eq!(array_subset.shape(), output_subset.shape()); let decoded_value = self .partial_decode(&[array_subset.clone()], options) @@ -728,7 +728,7 @@ pub trait ArrayToBytesCodecTraits: ArrayCodecTraits + core::fmt::Debug { output_subset: &ArraySubset, options: &CodecOptions, ) -> Result<(), CodecError> { - debug_assert!(output_subset.inbounds(output_shape)); + debug_assert!(output_subset.inbounds_shape(output_shape)); debug_assert_eq!( decoded_representation.num_elements(), output_subset.num_elements() diff --git a/zarrs/src/array_subset.rs b/zarrs/src/array_subset.rs index ac7b8cde..79730915 100644 --- a/zarrs/src/array_subset.rs +++ b/zarrs/src/array_subset.rs @@ -564,9 +564,26 @@ impl ArraySubset { } } - /// Returns true if the array subset is within the bounds of `array_shape`. + /// Returns true if this array subset is within the bounds of `subset`. #[must_use] - pub fn inbounds(&self, array_shape: &[u64]) -> bool { + pub fn inbounds(&self, subset: &ArraySubset) -> bool { + if self.dimensionality() != subset.dimensionality() { + return false; + } + + for (self_start, self_shape, other_start, other_shape) in + izip!(self.start(), self.shape(), subset.start(), subset.shape()) + { + if self_start < other_start || self_start + self_shape > other_start + other_shape { + return false; + } + } + true + } + + /// Returns true if the array subset is within the bounds of an `ArraySubset` with zero origin and a shape of `array_shape`. + #[must_use] + pub fn inbounds_shape(&self, array_shape: &[u64]) -> bool { if self.dimensionality() != array_shape.len() { return false; } @@ -646,9 +663,9 @@ mod tests { ArraySubset::new_with_ranges(&[0..4, 1..5]) ); assert!(array_subset0.relative_to(&[1, 1, 1]).is_err()); - assert!(array_subset0.inbounds(&[10, 10])); - assert!(!array_subset0.inbounds(&[2, 2])); - assert!(!array_subset0.inbounds(&[10, 10, 10])); + assert!(array_subset0.inbounds_shape(&[10, 10])); + assert!(!array_subset0.inbounds_shape(&[2, 2])); + assert!(!array_subset0.inbounds_shape(&[10, 10, 10])); assert_eq!(array_subset0.to_ranges(), vec![1..5, 2..6]); let array_subset2 = ArraySubset::new_with_ranges(&[3..6, 4..7, 0..1]); From e9ba2e9bf9700a78fa2bcb626adb918cae87c5c9 Mon Sep 17 00:00:00 2001 From: Lachlan Deakin Date: Sun, 19 Jan 2025 15:52:32 +1100 Subject: [PATCH 2/3] add inbounds tests --- zarrs/src/array_subset.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zarrs/src/array_subset.rs b/zarrs/src/array_subset.rs index 79730915..c7e65a31 100644 --- a/zarrs/src/array_subset.rs +++ b/zarrs/src/array_subset.rs @@ -666,6 +666,10 @@ mod tests { assert!(array_subset0.inbounds_shape(&[10, 10])); assert!(!array_subset0.inbounds_shape(&[2, 2])); assert!(!array_subset0.inbounds_shape(&[10, 10, 10])); + assert!(array_subset0.inbounds(&ArraySubset::new_with_ranges(&[0..6, 1..7]))); + assert!(array_subset0.inbounds(&ArraySubset::new_with_ranges(&[1..5, 2..6]))); + assert!(!array_subset0.inbounds(&ArraySubset::new_with_ranges(&[2..5, 2..6]))); + assert!(!array_subset0.inbounds(&ArraySubset::new_with_ranges(&[1..5, 2..5]))); assert_eq!(array_subset0.to_ranges(), vec![1..5, 2..6]); let array_subset2 = ArraySubset::new_with_ranges(&[3..6, 4..7, 0..1]); From e3f8841202a0ee1d9bd9cc9ee457b4957587432f Mon Sep 17 00:00:00 2001 From: Lachlan Deakin Date: Sun, 19 Jan 2025 15:55:56 +1100 Subject: [PATCH 3/3] another inbounds test --- zarrs/src/array_subset.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zarrs/src/array_subset.rs b/zarrs/src/array_subset.rs index c7e65a31..ef8846e0 100644 --- a/zarrs/src/array_subset.rs +++ b/zarrs/src/array_subset.rs @@ -670,6 +670,7 @@ mod tests { assert!(array_subset0.inbounds(&ArraySubset::new_with_ranges(&[1..5, 2..6]))); assert!(!array_subset0.inbounds(&ArraySubset::new_with_ranges(&[2..5, 2..6]))); assert!(!array_subset0.inbounds(&ArraySubset::new_with_ranges(&[1..5, 2..5]))); + assert!(!array_subset0.inbounds(&ArraySubset::new_with_ranges(&[2..5]))); assert_eq!(array_subset0.to_ranges(), vec![1..5, 2..6]); let array_subset2 = ArraySubset::new_with_ranges(&[3..6, 4..7, 0..1]);