Skip to content

Commit

Permalink
refactor!: change ArraySubset::inbounds to take another subset rath…
Browse files Browse the repository at this point in the history
…er than a shape (#134)
  • Loading branch information
LDeakin authored Jan 19, 2025
1 parent 862604b commit beecd30
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 13 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions zarrs/src/array/array_async_readable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits + 'static> Array<TStorage> {
options: &CodecOptions,
) -> Result<ArrayBytes<'_>, 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(),
Expand Down Expand Up @@ -783,7 +783,7 @@ impl<TStorage: ?Sized + AsyncReadableStorageTraits + 'static> Array<TStorage> {
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(),
Expand Down
4 changes: 2 additions & 2 deletions zarrs/src/array/array_sync_readable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> Array<TStorage> {
options: &CodecOptions,
) -> Result<ArrayBytes<'_>, 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(),
Expand Down Expand Up @@ -837,7 +837,7 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> Array<TStorage> {
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(),
Expand Down
2 changes: 1 addition & 1 deletion zarrs/src/array/chunk_cache/array_chunk_cache_ext_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl<TStorage: ?Sized + ReadableStorageTraits + 'static> ArrayChunkCacheExt<TSto
options: &CodecOptions,
) -> Result<ArrayBytes<'_>, 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(),
Expand Down
6 changes: 3 additions & 3 deletions zarrs/src/array/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,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)?
Expand Down Expand Up @@ -462,7 +462,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)
Expand Down Expand Up @@ -730,7 +730,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()
Expand Down
32 changes: 27 additions & 5 deletions zarrs/src/array_subset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -646,9 +663,14 @@ 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!(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!(!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]);
Expand Down

0 comments on commit beecd30

Please sign in to comment.