Skip to content

Commit c1cfc89

Browse files
BrianMichelltasansal
authored andcommitted
Implement full-dimension chunking support
1 parent d17a357 commit c1cfc89

File tree

2 files changed

+59
-4
lines changed

2 files changed

+59
-4
lines changed

src/mdio/builder/templates/base.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,29 @@ def coordinate_names(self) -> tuple[str, ...]:
148148
@property
149149
def full_chunk_shape(self) -> tuple[int, ...]:
150150
"""Returns the chunk shape for the variables."""
151-
return copy.deepcopy(self._var_chunk_shape)
151+
# If dimension sizes are not set yet, return the stored shape as-is
152+
if len(self._dim_sizes) != len(self._dim_names):
153+
return self._var_chunk_shape
154+
155+
# Expand -1 values to full dimension sizes
156+
return tuple(
157+
dim_size if chunk_size == -1 else chunk_size
158+
for chunk_size, dim_size in zip(self._var_chunk_shape, self._dim_sizes, strict=False)
159+
)
152160

153161
@full_chunk_shape.setter
154162
def full_chunk_shape(self, shape: tuple[int, ...]) -> None:
155163
"""Sets the chunk shape for the variables."""
156-
if len(shape) != len(self._dim_sizes):
157-
msg = f"Chunk shape {shape} does not match dimension sizes {self._dim_sizes}"
164+
if len(shape) != len(self._dim_names):
165+
msg = f"Chunk shape {shape} has {len(shape)} dimensions, expected {len(self._dim_names)}"
158166
raise ValueError(msg)
167+
168+
# Validate that all values are positive integers or -1
169+
for chunk_size in shape:
170+
if chunk_size != -1 and chunk_size <= 0:
171+
msg = f"Chunk size must be positive integer or -1, got {chunk_size}"
172+
raise ValueError(msg)
173+
159174
self._var_chunk_shape = shape
160175

161176
@property

tests/unit/v1/templates/test_seismic_templates.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,49 @@ def test_chunk_shape_assignment_exception(self) -> None:
2323
template = Seismic2DPostStackTemplate("time")
2424
template.build_dataset("test", (50, 50))
2525

26-
with pytest.raises(ValueError, match="Chunk shape.*does not match dimension sizes"):
26+
with pytest.raises(ValueError, match="Chunk shape.*has.*dimensions, expected"):
2727
template.full_chunk_shape = (32, 32, 32)
2828

29+
def test_chunk_shape_with_minus_one_before_build(self) -> None:
30+
"""Test that chunk shape can be set with -1 before build_dataset."""
31+
template = Seismic2DPostStackTemplate("time")
32+
33+
# Should be able to set chunk shape with -1 before build_dataset
34+
template.full_chunk_shape = (32, -1)
35+
36+
# Before build_dataset, getter should return unexpanded values
37+
assert template.full_chunk_shape == (32, -1)
38+
assert template._var_chunk_shape == (32, -1)
39+
40+
def test_chunk_shape_with_minus_one_after_build(self) -> None:
41+
"""Test that -1 values are expanded after build_dataset."""
42+
template = Seismic2DPostStackTemplate("time")
43+
template.full_chunk_shape = (32, -1)
44+
45+
# Build dataset with specific sizes
46+
template.build_dataset("test", (100, 200))
47+
48+
# After build_dataset, getter should expand -1 to dimension size
49+
assert template.full_chunk_shape == (32, 200)
50+
assert template._var_chunk_shape == (32, -1) # Internal storage unchanged
51+
52+
def test_chunk_shape_validation_invalid_values(self) -> None:
53+
"""Test that chunk shape setter rejects invalid values."""
54+
template = Seismic2DPostStackTemplate("time")
55+
template.build_dataset("test", (50, 50))
56+
57+
# Test rejection of 0
58+
with pytest.raises(ValueError, match="Chunk size must be positive integer or -1"):
59+
template.full_chunk_shape = (32, 0)
60+
61+
# Test rejection of negative values other than -1
62+
with pytest.raises(ValueError, match="Chunk size must be positive integer or -1"):
63+
template.full_chunk_shape = (32, -2)
64+
65+
# Test that positive values and -1 are accepted
66+
template.full_chunk_shape = (32, -1) # Should not raise
67+
template.full_chunk_shape = (32, 16) # Should not raise
68+
2969
def test_all_templates_inherit_from_abstract(self) -> None:
3070
"""Test that all concrete templates inherit from AbstractDatasetTemplate."""
3171
registry = TemplateRegistry()

0 commit comments

Comments
 (0)