Skip to content

Commit bd9e934

Browse files
committed
removed unnessasary resolveOutputShape calls
1 parent d2a80f6 commit bd9e934

2 files changed

Lines changed: 22 additions & 10 deletions

File tree

src/main/java/dev/zarr/zarrjava/v3/codec/core/ReshapeCodec.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,28 @@ public class ReshapeCodec extends ArrayArrayCodec implements Codec {
3939
@Nonnull
4040
public final Configuration configuration;
4141

42+
/**
43+
* The resolved (and validated) output chunk shape. It depends only on {@code arrayMetadata.chunkShape}
44+
* and is therefore computed once in {@link #setCoreArrayMetadata} rather than on every encode/decode.
45+
*/
46+
@JsonIgnore
47+
protected int[] outputChunkShape;
48+
4249
@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
4350
public ReshapeCodec(
4451
@Nonnull @JsonProperty(value = "configuration", required = true) Configuration configuration
4552
) {
4653
this.configuration = configuration;
4754
}
4855

56+
@Override
57+
public void setCoreArrayMetadata(ArrayMetadata.CoreArrayMetadata arrayMetadata) throws ZarrException {
58+
super.setCoreArrayMetadata(arrayMetadata);
59+
// Resolve and validate the output shape once, here, because it only changes when the
60+
// arrayMetadata changes. encode/decode/resolveArrayMetadata then reuse the cached value.
61+
this.outputChunkShape = resolveOutputShape(arrayMetadata.chunkShape);
62+
}
63+
4964
@Override
5065
public Array encode(Array chunkArray) throws ZarrException {
5166
int[] inputShape = arrayMetadata.chunkShape;
@@ -54,20 +69,18 @@ public Array encode(Array chunkArray) throws ZarrException {
5469
"reshape codec received an array of shape " + Arrays.toString(chunkArray.getShape())
5570
+ " but expected the chunk shape " + Arrays.toString(inputShape) + ".");
5671
}
57-
int[] outputShape = resolveOutputShape(inputShape);
5872
// Array.reshape copies the elements in lexicographical (C) order, hence ravel(B) == ravel(A)
5973
// even when the input array is a non-contiguous view.
60-
return chunkArray.reshape(outputShape);
74+
return chunkArray.reshape(outputChunkShape);
6175
}
6276

6377
@Override
6478
public Array decode(Array chunkArray) throws ZarrException {
6579
int[] inputShape = arrayMetadata.chunkShape;
66-
int[] outputShape = resolveOutputShape(inputShape);
67-
if (!Arrays.equals(chunkArray.getShape(), outputShape)) {
80+
if (!Arrays.equals(chunkArray.getShape(), outputChunkShape)) {
6881
throw new ZarrException(
6982
"reshape codec received an array of shape " + Arrays.toString(chunkArray.getShape())
70-
+ " but expected the reshaped shape " + Arrays.toString(outputShape) + ".");
83+
+ " but expected the reshaped shape " + Arrays.toString(outputChunkShape) + ".");
7184
}
7285
// Inverse operation: reshape back to the original chunk shape.
7386
return chunkArray.reshape(inputShape);
@@ -85,7 +98,6 @@ public ArrayMetadata.CoreArrayMetadata resolveArrayMetadata() throws ZarrExcepti
8598
super.resolveArrayMetadata();
8699

87100
int[] inputChunkShape = arrayMetadata.chunkShape;
88-
int[] outputChunkShape = resolveOutputShape(inputChunkShape);
89101

90102
// Derive the output (grid) array shape so that number of chunks along each output dimension is
91103
// consistent with the input. For a merged output dimension this is the product of the input

src/test/java/dev/zarr/zarrjava/ReshapeCodecTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,10 @@ static Stream<Arguments> invalidReshapes() {
150150

151151
@ParameterizedTest
152152
@MethodSource("invalidReshapes")
153-
public void testReshapeInvalidConfig(int[] inputShape, Object[] shape) throws ZarrException {
154-
ReshapeCodec codec = reshapeCodec(shape, inputShape);
155-
ucar.ma2.Array input = sequential(inputShape);
156-
assertThrows(ZarrException.class, () -> codec.encode(input));
153+
public void testReshapeInvalidConfig(int[] inputShape, Object[] shape) {
154+
// Invalid configurations are rejected eagerly when the array metadata is set (i.e. at pipeline
155+
// construction time), not lazily on the first encode/decode.
156+
assertThrows(ZarrException.class, () -> reshapeCodec(shape, inputShape));
157157
}
158158

159159
@Test

0 commit comments

Comments
 (0)