Skip to content

add reshape codec#82

Open
konstibob wants to merge 2 commits into
zarr-developers:mainfrom
konstibob:reshape_codec
Open

add reshape codec#82
konstibob wants to merge 2 commits into
zarr-developers:mainfrom
konstibob:reshape_codec

Conversation

@konstibob

@konstibob konstibob commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Implemented a spec-compliant ReshapeCodec zarr-developers/zarr-extensions#10 in zarr-java plus tests, all passing.

  • Codec: array→array reshape preserving C-order ravel; validates all spec rules (input_dims, -1, product invariant, strictly-increasing dims, alignment).
  • Tests: round-trips (merge/split/flatten/singletons/-1/high-dim→1D), metadata resolution, ~11 invalid-config edge cases, and end-to-end read/write (incl. combined with transpose).

split up functions in reshape to make more readable

split up resolveOutputShape

@normanrz normanrz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few minor notes.

"reshape codec received an array of shape " + Arrays.toString(chunkArray.getShape())
+ " but expected the chunk shape " + Arrays.toString(inputShape) + ".");
}
int[] outputShape = resolveOutputShape(inputShape);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The outputChunkShape is computed in every encode and decode call. It could be cached as a protected property of the class, because it is only changed when the arrayMetadata changes. You can override the setCoreArrayMetadata metadata, compute the outputChunkShape there, and use the property in encode and decode.

// decode is the inverse of encode.
ucar.ma2.Array decoded = codec.decode(encoded);
Assertions.assertArrayEquals(inputShape, decoded.getShape());
assert MAMath.equals(input, decoded);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assert keyword should not be used. Replace with:

Suggested change
assert MAMath.equals(input, decoded);
Assertions.assertTrue(MAMath.equals(input, decoded));

.withDataType(DataType.UINT32)
.withChunkShape(4, 5, 6, 3)
.withFillValue(0)
.withCodecs(c -> c.withReshape(new Object[]{new int[]{0, 1}, new int[]{2}, 3}).withZstd());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we landed #81, we should add a test that uses reshape + jpeg. Because reshape does not alter the byte-layout, this test doesn't do much. Still useful, though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants