From e04672926cbb964e0b64620d09dc4bd16d4cf51c Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 18:32:20 +0200 Subject: [PATCH 01/12] refactor(collection)!: rename DatasetCollection.create_cube to create Rename the in-memory template factory create_cube -> create (dropping the legacy 'cube'/datacube naming), align its parameter dataset_length -> time_length with the constructor, and fix the broken docstring. Update all call sites in src docstrings, tests, and docs. No deprecated alias. BREAKING CHANGE: DatasetCollection.create_cube is removed; use DatasetCollection.create(src, time_length=...) instead. --- docs/pyramids.drawio | 6 ++-- docs/tutorials/lazy/lazy-collection.md | 2 +- src/pyramids/dataset/collection.py | 30 +++++++++------- .../test_dataset_collection_unit.py | 34 +++++++++---------- tests/dataset/collection/test_meta.py | 6 ++-- tests/dataset/collection/test_plot_labels.py | 2 +- tests/dataset/collection/test_to_netcdf.py | 10 +++--- tests/dataset/plot/test_basemap_dispatch.py | 2 +- tests/e2e/test_e2e_workflows.py | 6 ++-- 9 files changed, 51 insertions(+), 47 deletions(-) diff --git a/docs/pyramids.drawio b/docs/pyramids.drawio index 809336fc1a..bf51407140 100644 --- a/docs/pyramids.drawio +++ b/docs/pyramids.drawio @@ -283,7 +283,7 @@ - + @@ -811,7 +811,7 @@ - + @@ -3982,7 +3982,7 @@ - + diff --git a/docs/tutorials/lazy/lazy-collection.md b/docs/tutorials/lazy/lazy-collection.md index 4b3c6588ee..05c50e006a 100644 --- a/docs/tutorials/lazy/lazy-collection.md +++ b/docs/tutorials/lazy/lazy-collection.md @@ -218,7 +218,7 @@ execution; useful when the store-write is one step of a larger graph. `collection.to_zarr` raises `RuntimeError` on a collection without a -`files` list (e.g. the legacy `create_cube(src, n)` path) — Zarr +`files` list (e.g. the legacy `create(src, n)` path) — Zarr writes need a source file per timestep. `ImportError` raised when the `[lazy]` extra is missing. diff --git a/src/pyramids/dataset/collection.py b/src/pyramids/dataset/collection.py index 7712fee8f5..c1e7f703b8 100644 --- a/src/pyramids/dataset/collection.py +++ b/src/pyramids/dataset/collection.py @@ -865,21 +865,25 @@ def columns(self): return self._base.columns @classmethod - def create_cube(cls, src: Dataset, dataset_length: int) -> DatasetCollection: - """Create DatasetCollection. + def create(cls, src: Dataset, time_length: int) -> DatasetCollection: + """Build an in-memory collection from a template raster. - - Create DatasetCollection from a sample raster and + Creates a scaffold of ``time_length`` timesteps that all share + ``src``'s geobox (CRS, geotransform, dtype) and has no backing files — + the values are filled in memory. Contrast with the data-source readers + :meth:`from_files`, :meth:`from_stac`, and :meth:`from_zarr`. Args: - src (Dataset): - Raster object. - dataset_length (int): - Length of the dataset. + src: Template :class:`~pyramids.dataset.Dataset` supplying the + geobox; it also serves as the single timestep until values are + set. + time_length: Number of timesteps in the collection. Returns: - DatasetCollection: DatasetCollection object. + DatasetCollection: An in-memory collection whose ``files`` is + ``None``. """ - return cls(src, dataset_length) + return cls(src, time_length) def groupby(self, time_labels) -> _GroupedCollection: """Group time steps by per-timestep label. @@ -1112,7 +1116,7 @@ def data(self) -> Any: ImportError: If the optional `dask` extra is not installed. RuntimeError: If the collection was constructed without a - `files` list (legacy `create_cube` path). + `files` list (the in-memory `create` path). """ if self._zarr_store is None and (self._files is None or len(self._files) == 0): raise RuntimeError( @@ -2842,7 +2846,7 @@ def to_file( >>> src = Dataset.create_from_array( ... np.ones((5, 5), dtype="float32"), top_left_corner=(0, 5), cell_size=1.0, epsg=4326, ... ) - >>> collection = DatasetCollection.create_cube(src, 3) + >>> collection = DatasetCollection.create(src, 3) >>> out_dir = tempfile.mkdtemp() >>> collection.to_file(out_dir) >>> sorted(os.listdir(out_dir)) @@ -2858,7 +2862,7 @@ def to_file( >>> src = Dataset.create_from_array( ... np.full((4, 4), 7.0, dtype="float32"), top_left_corner=(0, 4), cell_size=1.0, epsg=4326, ... ) - >>> collection = DatasetCollection.create_cube(src, 2) + >>> collection = DatasetCollection.create(src, 2) >>> out_dir = tempfile.mkdtemp() >>> paths = [os.path.join(out_dir, f"slice_{i}.tif") for i in range(2)] >>> collection.to_file(paths) @@ -3161,7 +3165,7 @@ def crop( >>> mask = Dataset.create_from_array( ... np.ones((10, 10), dtype="int16"), top_left_corner=(0, 0), cell_size=0.05, epsg=4326, ... ) - >>> collection = DatasetCollection.create_cube(mask, 3) + >>> collection = DatasetCollection.create(mask, 3) >>> cropped = collection.crop(mask=mask) >>> cropped.time_length 3 diff --git a/tests/dataset/collection/test_dataset_collection_unit.py b/tests/dataset/collection/test_dataset_collection_unit.py index 467e1eb3ad..bcd2b67f29 100644 --- a/tests/dataset/collection/test_dataset_collection_unit.py +++ b/tests/dataset/collection/test_dataset_collection_unit.py @@ -2,7 +2,7 @@ Targets untested / low-coverage code paths in ``pyramids.dataset.collection``, including: -- ``create_cube`` classmethod +- ``create`` classmethod - ``merge`` static method (via a temp-file round-trip) - ``apply`` method with ufunc - ``overlay`` with classes @@ -64,36 +64,36 @@ def base_dataset() -> Dataset: @pytest.fixture() def cube_with_values(base_dataset: Dataset) -> DatasetCollection: """A DatasetCollection with 3 time steps and pre-set values.""" - md = DatasetCollection.create_cube(base_dataset, dataset_length=3) + md = DatasetCollection.create(base_dataset, time_length=3) values = np.arange(3 * 5 * 6, dtype=np.float64).reshape(3, 5, 6) md.values = values return md class TestCreateCube: - """Tests for the ``create_cube`` classmethod.""" + """Tests for the ``create`` classmethod.""" def test_returns_dataset_collection(self, base_dataset: Dataset): - """create_cube should return a DatasetCollection instance.""" - md = DatasetCollection.create_cube(base_dataset, dataset_length=4) + """create should return a DatasetCollection instance.""" + md = DatasetCollection.create(base_dataset, time_length=4) assert isinstance(md, DatasetCollection), ( f"Expected DatasetCollection, got {type(md)}" ) def test_time_length_matches(self, base_dataset: Dataset): - """The time_length should match the given dataset_length.""" - md = DatasetCollection.create_cube(base_dataset, dataset_length=7) + """The time_length should match the given time_length.""" + md = DatasetCollection.create(base_dataset, time_length=7) assert md.time_length == 7, f"Expected time_length=7, got {md.time_length}" def test_base_is_same_dataset(self, base_dataset: Dataset): """The base property should reference the provided Dataset.""" - md = DatasetCollection.create_cube(base_dataset, dataset_length=1) + md = DatasetCollection.create(base_dataset, time_length=1) assert md.base is base_dataset, "base should be the original Dataset" def test_files_is_none(self, base_dataset: Dataset): - """create_cube does not set files so it should be None.""" - md = DatasetCollection.create_cube(base_dataset, dataset_length=2) - assert md.files is None, "files should be None for create_cube" + """create does not set files so it should be None.""" + md = DatasetCollection.create(base_dataset, time_length=2) + assert md.files is None, "files should be None for create" class TestStringRepresentation: @@ -562,7 +562,7 @@ def test_apply_numpy_ufunc(self, base_dataset: Dataset): new ``DatasetCollection`` instead of mutating ``self``. The assertion runs against the returned collection's ``values``. """ - md = DatasetCollection.create_cube(base_dataset, dataset_length=2) + md = DatasetCollection.create(base_dataset, time_length=2) values = np.full((2, 5, 6), -5.0) values[:, 0, -1] = -9999.0 # set nodata in one cell md.values = values @@ -579,7 +579,7 @@ def test_apply_custom_ufunc(self, base_dataset: Dataset): After the L-3 refactor ``apply`` is out-of-place — see :meth:`test_apply_numpy_ufunc`. """ - md = DatasetCollection.create_cube(base_dataset, dataset_length=2) + md = DatasetCollection.create(base_dataset, time_length=2) values = np.full((2, 5, 6), 10.0) values[:, 0, 0] = -9999.0 md.values = values @@ -732,10 +732,10 @@ def test_to_file_single_timestep(self, base_dataset: Dataset): """A one-timestep collection writes exactly one file with the base pixels. Test scenario: - ``create_cube(base, 1)`` written to a directory yields a single + ``create(base, 1)`` written to a directory yields a single ``0.tif`` whose pixels equal the base template. """ - cube = DatasetCollection.create_cube(base_dataset, dataset_length=1) + cube = DatasetCollection.create(base_dataset, time_length=1) tmp_dir = Path(tempfile.mkdtemp()) out_dir = tmp_dir / "single" try: @@ -757,7 +757,7 @@ def test_to_file_list_infers_driver_from_path_extension( driver is ``geotiff`` — the pre-streaming extension-inference contract. Guards against silently writing GeoTIFF bytes into ``.asc``-named files. """ - cube = DatasetCollection.create_cube(base_dataset, dataset_length=2) + cube = DatasetCollection.create(base_dataset, time_length=2) tmp_dir = Path(tempfile.mkdtemp()) paths = [tmp_dir / f"grid_{i}.asc" for i in range(2)] try: @@ -796,7 +796,7 @@ def test_to_file_preserves_color_table(self): ct.SetColorEntry(2, (0, 255, 0, 255)) band.SetRasterColorTable(ct) band.WriteArray(np.array([[1, 2, 1], [2, 1, 2], [1, 2, 1]], dtype=np.uint8)) - cube = DatasetCollection.create_cube(src, dataset_length=1) + cube = DatasetCollection.create(src, time_length=1) tmp_dir = Path(tempfile.mkdtemp()) try: cube.to_file(tmp_dir / "paletted") diff --git a/tests/dataset/collection/test_meta.py b/tests/dataset/collection/test_meta.py index 65509b40ea..9dcfdec8d2 100644 --- a/tests/dataset/collection/test_meta.py +++ b/tests/dataset/collection/test_meta.py @@ -1,7 +1,7 @@ """Tests for :class:`DatasetCollection` RasterMeta refactor. Backwards-compatible refactor: existing ``DatasetCollection(src, -time_length)` + `create_cube`` paths are unchanged. Two additions +time_length)` + `create`` paths are unchanged. Two additions under test: * `.meta` property returns a :class:`RasterMeta` snapshot derived @@ -84,9 +84,9 @@ def test_meta_pickle_roundtrip(self, template_file): class TestBackwardsCompat: - def test_create_cube_still_works(self, template_file): + def test_create_still_works(self, template_file): src = Dataset.read_file(template_file) - collection = DatasetCollection.create_cube(src, dataset_length=5) + collection = DatasetCollection.create(src, time_length=5) assert collection.time_length == 5 assert collection.meta.rows == 4 diff --git a/tests/dataset/collection/test_plot_labels.py b/tests/dataset/collection/test_plot_labels.py index b83ffaa548..d5fe9ca8a0 100644 --- a/tests/dataset/collection/test_plot_labels.py +++ b/tests/dataset/collection/test_plot_labels.py @@ -34,7 +34,7 @@ def _collection(count: int = 3, bands: int = 1) -> DatasetCollection: src = Dataset.create_from_array( arr, top_left_corner=(0, 0), cell_size=0.05, epsg=4326 ) - return DatasetCollection.create_cube(src, count) + return DatasetCollection.create(src, count) def _dated_files(tmp_path, years=(2000, 2001, 2002)) -> list[str]: diff --git a/tests/dataset/collection/test_to_netcdf.py b/tests/dataset/collection/test_to_netcdf.py index cf24ce5d8d..c7bd99da31 100644 --- a/tests/dataset/collection/test_to_netcdf.py +++ b/tests/dataset/collection/test_to_netcdf.py @@ -670,16 +670,16 @@ def test_nodata_round_trips_var_per_band_false(self, tmp_path): class TestToNetcdfNoFilesPath: - """Support for collections that have no ``_files`` (e.g. ``create_cube``).""" + """Support for collections that have no ``_files`` (e.g. ``create``).""" - def test_create_cube_collection_writes_successfully(self, tmp_path): - """A ``create_cube``-backed collection (no file list) can still be written. + def test_create_collection_writes_successfully(self, tmp_path): + """A ``create``-backed collection (no file list) can still be written. Args: tmp_path: pytest temp directory. Test scenario: - Build via :meth:`DatasetCollection.create_cube` (legacy path + Build via :meth:`DatasetCollection.create` (legacy path that stamps a single ``Dataset`` repeated T times) — expected: the writer materialises from ``self.datasets`` and produces a real file. @@ -694,7 +694,7 @@ def test_create_cube_collection_writes_successfully(self, tmp_path): path=src_path, ).close() src = Dataset.read_file(src_path) - col = DatasetCollection.create_cube(src, 3) + col = DatasetCollection.create(src, 3) out = tmp_path / "nf.nc" col.to_netcdf(str(out)) assert out.exists(), "no-files write did not produce a file" diff --git a/tests/dataset/plot/test_basemap_dispatch.py b/tests/dataset/plot/test_basemap_dispatch.py index cc434f2473..55d792f301 100644 --- a/tests/dataset/plot/test_basemap_dispatch.py +++ b/tests/dataset/plot/test_basemap_dispatch.py @@ -229,7 +229,7 @@ def test_collection_plot_draws_tile_basemap_end_to_end(self): class-level uniformity the base `plot`/`animate` API promises: a web-tile basemap works on `DatasetCollection` exactly as it does on `Dataset`. """ - cube = DatasetCollection.create_cube(self._dataset(), 3) + cube = DatasetCollection.create(self._dataset(), 3) with patch("pyramids.basemap.basemap.add_basemap") as mock_add: cube.plot(band=0, basemap="CartoDB.Positron") assert mock_add.called, "collection animate path must draw the web-tile basemap" diff --git a/tests/e2e/test_e2e_workflows.py b/tests/e2e/test_e2e_workflows.py index e7ea896c83..290abf0d05 100644 --- a/tests/e2e/test_e2e_workflows.py +++ b/tests/e2e/test_e2e_workflows.py @@ -106,7 +106,7 @@ def test_save_and_reload(self): time_steps = 3 base = _make_dataset(rows=rows, cols=cols, fill_value=1.0) - md = DatasetCollection.create_cube(base, dataset_length=time_steps) + md = DatasetCollection.create(base, time_length=time_steps) values = ( np.random.default_rng(0).random((time_steps, rows, cols)).astype(np.float64) ) @@ -579,7 +579,7 @@ def test_apply_then_iterate(self): time_steps = 4 base = _make_dataset(rows=rows, cols=cols, fill_value=10.0) - md = DatasetCollection.create_cube(base, dataset_length=time_steps) + md = DatasetCollection.create(base, time_length=time_steps) # Fill with known values: each time step has value = step_index + 1 values = np.zeros((time_steps, rows, cols), dtype=np.float64) @@ -605,7 +605,7 @@ def test_head_tail_first_last(self): time_steps = 6 base = _make_dataset(rows=rows, cols=cols) - md = DatasetCollection.create_cube(base, dataset_length=time_steps) + md = DatasetCollection.create(base, time_length=time_steps) values = np.random.default_rng(0).random((time_steps, rows, cols)) md.values = values From 5643b067880c450d5723ac4c3542233b9079e508 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 18:55:03 +0200 Subject: [PATCH 02/12] refactor(collection)!: rename DatasetCollection.create to from_dataset Align the in-memory template factory with the from_* reader family and avoid the name collision with Dataset.create: create -> from_dataset (same (src, time_length) signature). Update all call sites in the src docstrings, tests, and docs. Dataset.create is unaffected. BREAKING CHANGE: DatasetCollection.create is renamed to from_dataset; use DatasetCollection.from_dataset(src, time_length=...). --- docs/pyramids.drawio | 6 +++--- docs/tutorials/lazy/lazy-collection.md | 2 +- src/pyramids/dataset/collection.py | 12 +++++------ .../test_dataset_collection_unit.py | 20 +++++++++---------- tests/dataset/collection/test_meta.py | 2 +- tests/dataset/collection/test_plot_labels.py | 2 +- tests/dataset/collection/test_to_netcdf.py | 2 +- tests/dataset/plot/test_basemap_dispatch.py | 2 +- tests/e2e/test_e2e_workflows.py | 6 +++--- 9 files changed, 27 insertions(+), 27 deletions(-) diff --git a/docs/pyramids.drawio b/docs/pyramids.drawio index bf51407140..3135742f34 100644 --- a/docs/pyramids.drawio +++ b/docs/pyramids.drawio @@ -283,7 +283,7 @@ - + @@ -811,7 +811,7 @@ - + @@ -3982,7 +3982,7 @@ - + diff --git a/docs/tutorials/lazy/lazy-collection.md b/docs/tutorials/lazy/lazy-collection.md index 05c50e006a..d07cd222ee 100644 --- a/docs/tutorials/lazy/lazy-collection.md +++ b/docs/tutorials/lazy/lazy-collection.md @@ -218,7 +218,7 @@ execution; useful when the store-write is one step of a larger graph. `collection.to_zarr` raises `RuntimeError` on a collection without a -`files` list (e.g. the legacy `create(src, n)` path) — Zarr +`files` list (e.g. the in-memory `from_dataset(src, n)` path) — Zarr writes need a source file per timestep. `ImportError` raised when the `[lazy]` extra is missing. diff --git a/src/pyramids/dataset/collection.py b/src/pyramids/dataset/collection.py index c1e7f703b8..389836ce81 100644 --- a/src/pyramids/dataset/collection.py +++ b/src/pyramids/dataset/collection.py @@ -865,8 +865,8 @@ def columns(self): return self._base.columns @classmethod - def create(cls, src: Dataset, time_length: int) -> DatasetCollection: - """Build an in-memory collection from a template raster. + def from_dataset(cls, src: Dataset, time_length: int) -> DatasetCollection: + """Build an in-memory collection from a template Dataset. Creates a scaffold of ``time_length`` timesteps that all share ``src``'s geobox (CRS, geotransform, dtype) and has no backing files — @@ -1116,7 +1116,7 @@ def data(self) -> Any: ImportError: If the optional `dask` extra is not installed. RuntimeError: If the collection was constructed without a - `files` list (the in-memory `create` path). + `files` list (the in-memory `from_dataset` path). """ if self._zarr_store is None and (self._files is None or len(self._files) == 0): raise RuntimeError( @@ -2846,7 +2846,7 @@ def to_file( >>> src = Dataset.create_from_array( ... np.ones((5, 5), dtype="float32"), top_left_corner=(0, 5), cell_size=1.0, epsg=4326, ... ) - >>> collection = DatasetCollection.create(src, 3) + >>> collection = DatasetCollection.from_dataset(src, 3) >>> out_dir = tempfile.mkdtemp() >>> collection.to_file(out_dir) >>> sorted(os.listdir(out_dir)) @@ -2862,7 +2862,7 @@ def to_file( >>> src = Dataset.create_from_array( ... np.full((4, 4), 7.0, dtype="float32"), top_left_corner=(0, 4), cell_size=1.0, epsg=4326, ... ) - >>> collection = DatasetCollection.create(src, 2) + >>> collection = DatasetCollection.from_dataset(src, 2) >>> out_dir = tempfile.mkdtemp() >>> paths = [os.path.join(out_dir, f"slice_{i}.tif") for i in range(2)] >>> collection.to_file(paths) @@ -3165,7 +3165,7 @@ def crop( >>> mask = Dataset.create_from_array( ... np.ones((10, 10), dtype="int16"), top_left_corner=(0, 0), cell_size=0.05, epsg=4326, ... ) - >>> collection = DatasetCollection.create(mask, 3) + >>> collection = DatasetCollection.from_dataset(mask, 3) >>> cropped = collection.crop(mask=mask) >>> cropped.time_length 3 diff --git a/tests/dataset/collection/test_dataset_collection_unit.py b/tests/dataset/collection/test_dataset_collection_unit.py index bcd2b67f29..5c427b9b6e 100644 --- a/tests/dataset/collection/test_dataset_collection_unit.py +++ b/tests/dataset/collection/test_dataset_collection_unit.py @@ -64,7 +64,7 @@ def base_dataset() -> Dataset: @pytest.fixture() def cube_with_values(base_dataset: Dataset) -> DatasetCollection: """A DatasetCollection with 3 time steps and pre-set values.""" - md = DatasetCollection.create(base_dataset, time_length=3) + md = DatasetCollection.from_dataset(base_dataset, time_length=3) values = np.arange(3 * 5 * 6, dtype=np.float64).reshape(3, 5, 6) md.values = values return md @@ -75,24 +75,24 @@ class TestCreateCube: def test_returns_dataset_collection(self, base_dataset: Dataset): """create should return a DatasetCollection instance.""" - md = DatasetCollection.create(base_dataset, time_length=4) + md = DatasetCollection.from_dataset(base_dataset, time_length=4) assert isinstance(md, DatasetCollection), ( f"Expected DatasetCollection, got {type(md)}" ) def test_time_length_matches(self, base_dataset: Dataset): """The time_length should match the given time_length.""" - md = DatasetCollection.create(base_dataset, time_length=7) + md = DatasetCollection.from_dataset(base_dataset, time_length=7) assert md.time_length == 7, f"Expected time_length=7, got {md.time_length}" def test_base_is_same_dataset(self, base_dataset: Dataset): """The base property should reference the provided Dataset.""" - md = DatasetCollection.create(base_dataset, time_length=1) + md = DatasetCollection.from_dataset(base_dataset, time_length=1) assert md.base is base_dataset, "base should be the original Dataset" def test_files_is_none(self, base_dataset: Dataset): """create does not set files so it should be None.""" - md = DatasetCollection.create(base_dataset, time_length=2) + md = DatasetCollection.from_dataset(base_dataset, time_length=2) assert md.files is None, "files should be None for create" @@ -562,7 +562,7 @@ def test_apply_numpy_ufunc(self, base_dataset: Dataset): new ``DatasetCollection`` instead of mutating ``self``. The assertion runs against the returned collection's ``values``. """ - md = DatasetCollection.create(base_dataset, time_length=2) + md = DatasetCollection.from_dataset(base_dataset, time_length=2) values = np.full((2, 5, 6), -5.0) values[:, 0, -1] = -9999.0 # set nodata in one cell md.values = values @@ -579,7 +579,7 @@ def test_apply_custom_ufunc(self, base_dataset: Dataset): After the L-3 refactor ``apply`` is out-of-place — see :meth:`test_apply_numpy_ufunc`. """ - md = DatasetCollection.create(base_dataset, time_length=2) + md = DatasetCollection.from_dataset(base_dataset, time_length=2) values = np.full((2, 5, 6), 10.0) values[:, 0, 0] = -9999.0 md.values = values @@ -735,7 +735,7 @@ def test_to_file_single_timestep(self, base_dataset: Dataset): ``create(base, 1)`` written to a directory yields a single ``0.tif`` whose pixels equal the base template. """ - cube = DatasetCollection.create(base_dataset, time_length=1) + cube = DatasetCollection.from_dataset(base_dataset, time_length=1) tmp_dir = Path(tempfile.mkdtemp()) out_dir = tmp_dir / "single" try: @@ -757,7 +757,7 @@ def test_to_file_list_infers_driver_from_path_extension( driver is ``geotiff`` — the pre-streaming extension-inference contract. Guards against silently writing GeoTIFF bytes into ``.asc``-named files. """ - cube = DatasetCollection.create(base_dataset, time_length=2) + cube = DatasetCollection.from_dataset(base_dataset, time_length=2) tmp_dir = Path(tempfile.mkdtemp()) paths = [tmp_dir / f"grid_{i}.asc" for i in range(2)] try: @@ -796,7 +796,7 @@ def test_to_file_preserves_color_table(self): ct.SetColorEntry(2, (0, 255, 0, 255)) band.SetRasterColorTable(ct) band.WriteArray(np.array([[1, 2, 1], [2, 1, 2], [1, 2, 1]], dtype=np.uint8)) - cube = DatasetCollection.create(src, time_length=1) + cube = DatasetCollection.from_dataset(src, time_length=1) tmp_dir = Path(tempfile.mkdtemp()) try: cube.to_file(tmp_dir / "paletted") diff --git a/tests/dataset/collection/test_meta.py b/tests/dataset/collection/test_meta.py index 9dcfdec8d2..6ca82daab9 100644 --- a/tests/dataset/collection/test_meta.py +++ b/tests/dataset/collection/test_meta.py @@ -86,7 +86,7 @@ def test_meta_pickle_roundtrip(self, template_file): class TestBackwardsCompat: def test_create_still_works(self, template_file): src = Dataset.read_file(template_file) - collection = DatasetCollection.create(src, time_length=5) + collection = DatasetCollection.from_dataset(src, time_length=5) assert collection.time_length == 5 assert collection.meta.rows == 4 diff --git a/tests/dataset/collection/test_plot_labels.py b/tests/dataset/collection/test_plot_labels.py index d5fe9ca8a0..b09cb04ef6 100644 --- a/tests/dataset/collection/test_plot_labels.py +++ b/tests/dataset/collection/test_plot_labels.py @@ -34,7 +34,7 @@ def _collection(count: int = 3, bands: int = 1) -> DatasetCollection: src = Dataset.create_from_array( arr, top_left_corner=(0, 0), cell_size=0.05, epsg=4326 ) - return DatasetCollection.create(src, count) + return DatasetCollection.from_dataset(src, count) def _dated_files(tmp_path, years=(2000, 2001, 2002)) -> list[str]: diff --git a/tests/dataset/collection/test_to_netcdf.py b/tests/dataset/collection/test_to_netcdf.py index c7bd99da31..cec3392db2 100644 --- a/tests/dataset/collection/test_to_netcdf.py +++ b/tests/dataset/collection/test_to_netcdf.py @@ -694,7 +694,7 @@ def test_create_collection_writes_successfully(self, tmp_path): path=src_path, ).close() src = Dataset.read_file(src_path) - col = DatasetCollection.create(src, 3) + col = DatasetCollection.from_dataset(src, 3) out = tmp_path / "nf.nc" col.to_netcdf(str(out)) assert out.exists(), "no-files write did not produce a file" diff --git a/tests/dataset/plot/test_basemap_dispatch.py b/tests/dataset/plot/test_basemap_dispatch.py index 55d792f301..dcf4e42f77 100644 --- a/tests/dataset/plot/test_basemap_dispatch.py +++ b/tests/dataset/plot/test_basemap_dispatch.py @@ -229,7 +229,7 @@ def test_collection_plot_draws_tile_basemap_end_to_end(self): class-level uniformity the base `plot`/`animate` API promises: a web-tile basemap works on `DatasetCollection` exactly as it does on `Dataset`. """ - cube = DatasetCollection.create(self._dataset(), 3) + cube = DatasetCollection.from_dataset(self._dataset(), 3) with patch("pyramids.basemap.basemap.add_basemap") as mock_add: cube.plot(band=0, basemap="CartoDB.Positron") assert mock_add.called, "collection animate path must draw the web-tile basemap" diff --git a/tests/e2e/test_e2e_workflows.py b/tests/e2e/test_e2e_workflows.py index 290abf0d05..4788e7e647 100644 --- a/tests/e2e/test_e2e_workflows.py +++ b/tests/e2e/test_e2e_workflows.py @@ -106,7 +106,7 @@ def test_save_and_reload(self): time_steps = 3 base = _make_dataset(rows=rows, cols=cols, fill_value=1.0) - md = DatasetCollection.create(base, time_length=time_steps) + md = DatasetCollection.from_dataset(base, time_length=time_steps) values = ( np.random.default_rng(0).random((time_steps, rows, cols)).astype(np.float64) ) @@ -579,7 +579,7 @@ def test_apply_then_iterate(self): time_steps = 4 base = _make_dataset(rows=rows, cols=cols, fill_value=10.0) - md = DatasetCollection.create(base, time_length=time_steps) + md = DatasetCollection.from_dataset(base, time_length=time_steps) # Fill with known values: each time step has value = step_index + 1 values = np.zeros((time_steps, rows, cols), dtype=np.float64) @@ -605,7 +605,7 @@ def test_head_tail_first_last(self): time_steps = 6 base = _make_dataset(rows=rows, cols=cols) - md = DatasetCollection.create(base, time_length=time_steps) + md = DatasetCollection.from_dataset(base, time_length=time_steps) values = np.random.default_rng(0).random((time_steps, rows, cols)) md.values = values From 54e6f04a5278390da4160894739295c6fab23749 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 19:06:55 +0200 Subject: [PATCH 03/12] docs(collection): add examples and see-also to from_dataset Match the from_* reader docstring style: add two +SKIP examples showing the in-memory scaffold (time_length, files is None) and a See Also cross-referencing from_files/from_zarr/from_stac. --- src/pyramids/dataset/collection.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/pyramids/dataset/collection.py b/src/pyramids/dataset/collection.py index 389836ce81..3686939762 100644 --- a/src/pyramids/dataset/collection.py +++ b/src/pyramids/dataset/collection.py @@ -882,6 +882,33 @@ def from_dataset(cls, src: Dataset, time_length: int) -> DatasetCollection: Returns: DatasetCollection: An in-memory collection whose ``files`` is ``None``. + + Examples: + - Scaffold a 3-timestep collection from a template raster: + + ```python + >>> from pyramids.dataset import Dataset, DatasetCollection + >>> template = Dataset.read_file("dem.tif") # doctest: +SKIP + >>> cube = DatasetCollection.from_dataset(template, 3) # doctest: +SKIP + >>> cube.time_length # doctest: +SKIP + 3 + + ``` + - The scaffold is in memory, so it has no backing files: + + ```python + >>> from pyramids.dataset import Dataset, DatasetCollection + >>> template = Dataset.read_file("dem.tif") # doctest: +SKIP + >>> cube = DatasetCollection.from_dataset(template, 5) # doctest: +SKIP + >>> cube.files is None # doctest: +SKIP + True + + ``` + + See Also: + from_files: Build a collection from rasters on disk. + from_zarr: Build a collection from a Zarr store. + from_stac: Build a collection from a STAC query. """ return cls(src, time_length) From f0eab7db962d00a80d5f323e38593bdc7fbc5cf6 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 19:19:01 +0200 Subject: [PATCH 04/12] docs(collection): add classDiagram views of constructors and properties Add an 'API at a glance' section to the collection reference with two mermaid classDiagram blocks: one for the from_* constructors (all classmethods, returning DatasetCollection) and one for the public properties, with notes on settable vs read-only and nullable fields. --- docs/reference/dataset/collection.md | 51 ++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/docs/reference/dataset/collection.md b/docs/reference/dataset/collection.md index 0caf3f67ab..3fd120c890 100644 --- a/docs/reference/dataset/collection.md +++ b/docs/reference/dataset/collection.md @@ -16,6 +16,57 @@ flowchart LR DC --> WR["write
to_file · to_cog_stack
to_zarr · to_netcdf · to_kerchunk"] ``` +## API at a glance + +Two `classDiagram` views of the public surface: the **constructors** (every `@classmethod`, all returning a +`DatasetCollection`) and the **properties**. Exact parameter types and defaults live in the auto-generated +reference below; the diagrams show parameters by name to stay readable. + +### Constructors (classmethods) + +```mermaid +classDiagram + class DatasetCollection { + +from_dataset(src, time_length) DatasetCollection + +from_files(files, glob, date_format, date_regex, start, end, meta, gdal_env, validate) DatasetCollection + +from_stac(items, asset, patch_url, bbox, max_items, signer, align, skip_missing, groupby, like, crs, resolution, bounds, anchor) DatasetCollection + +from_point(lat, lon, collection, bands, start_date, end_date, edge_size, resolution, units, stac, query, signer, align) DatasetCollection + +from_zarr(store, storage_options) DatasetCollection + +from_archive(url_or_path, kind, member_glob, meta) DatasetCollection + +read_multiple_files(path, with_order, regex_string, date, file_name_data_fmt, start, end, fmt, glob) DatasetCollection + } +``` + +`from_dataset` builds an in-memory scaffold from a template `Dataset`; the other constructors read from a data +source — a folder or explicit list (`from_files`), a STAC search (`from_stac`), a single STAC point query +(`from_point`), a Zarr store (`from_zarr`), or a zip/tar archive (`from_archive`). Every constructor after +`from_dataset` takes its non-`self` extras as keyword-only arguments (see the reference for the exact `*` +boundary). `read_multiple_files` is **deprecated** — use `from_files`. + +### Properties + +```mermaid +classDiagram + class DatasetCollection { + +list~Dataset~ datasets + +Dataset base + +list~str~ files + +int time_length + +list time + +int rows + +int columns + +tuple shape + +Array data + +RasterMeta meta + +NDArray values + } +``` + +`time` and `values` are settable; every other property is read-only. `files` is `None` for an in-memory +collection and `time` is `None` until a time axis is assigned. `data` is the Dask array over +`(time, bands, rows, cols)` (Path B) and requires a file-backed collection; `values` materialises the cube +eagerly through Path A. + ## The two paths The class operates through **two distinct backing paths**, each serving a From 8374c0db234e4cc42614c6a338a13f7637af4ef7 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 19:55:35 +0200 Subject: [PATCH 05/12] refactor(collection): rename from_dataset param src to dataset; clarify from_stac bbox vs bounds Rename the from_dataset template parameter 'src' to the more descriptive 'dataset' (echoes the method name and its type); all call sites pass it positionally, so no caller is affected. Update the classDiagram signature to match. Sharpen the from_stac docstring so the two (minx, miny, maxx, maxy) tuples are no longer confusable: bbox is the input filter in lon/lat that selects which STAC items are read, while bounds is the output grid extent in the target CRS. Note the divergence from odc-stac (where bbox sets the output extent). --- docs/reference/dataset/collection.md | 2 +- src/pyramids/dataset/collection.py | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/docs/reference/dataset/collection.md b/docs/reference/dataset/collection.md index 3fd120c890..96dbd1aba5 100644 --- a/docs/reference/dataset/collection.md +++ b/docs/reference/dataset/collection.md @@ -27,7 +27,7 @@ reference below; the diagrams show parameters by name to stay readable. ```mermaid classDiagram class DatasetCollection { - +from_dataset(src, time_length) DatasetCollection + +from_dataset(dataset, time_length) DatasetCollection +from_files(files, glob, date_format, date_regex, start, end, meta, gdal_env, validate) DatasetCollection +from_stac(items, asset, patch_url, bbox, max_items, signer, align, skip_missing, groupby, like, crs, resolution, bounds, anchor) DatasetCollection +from_point(lat, lon, collection, bands, start_date, end_date, edge_size, resolution, units, stac, query, signer, align) DatasetCollection diff --git a/src/pyramids/dataset/collection.py b/src/pyramids/dataset/collection.py index 3686939762..a8cdd43d81 100644 --- a/src/pyramids/dataset/collection.py +++ b/src/pyramids/dataset/collection.py @@ -865,16 +865,16 @@ def columns(self): return self._base.columns @classmethod - def from_dataset(cls, src: Dataset, time_length: int) -> DatasetCollection: + def from_dataset(cls, dataset: Dataset, time_length: int) -> DatasetCollection: """Build an in-memory collection from a template Dataset. Creates a scaffold of ``time_length`` timesteps that all share - ``src``'s geobox (CRS, geotransform, dtype) and has no backing files — - the values are filled in memory. Contrast with the data-source readers - :meth:`from_files`, :meth:`from_stac`, and :meth:`from_zarr`. + ``dataset``'s geobox (CRS, geotransform, dtype) and has no backing + files — the values are filled in memory. Contrast with the data-source + readers :meth:`from_files`, :meth:`from_stac`, and :meth:`from_zarr`. Args: - src: Template :class:`~pyramids.dataset.Dataset` supplying the + dataset: Template :class:`~pyramids.dataset.Dataset` supplying the geobox; it also serves as the single timestep until values are set. time_length: Number of timesteps in the collection. @@ -910,7 +910,7 @@ def from_dataset(cls, src: Dataset, time_length: int) -> DatasetCollection: from_zarr: Build a collection from a Zarr store. from_stac: Build a collection from a STAC query. """ - return cls(src, time_length) + return cls(dataset, time_length) def groupby(self, time_labels) -> _GroupedCollection: """Group time steps by per-timestep label. @@ -1695,9 +1695,12 @@ def from_stac( order). patch_url: Optional low-level callable rewriting each href (runs before `signer`). - bbox: M6 — optional `(minx, miny, maxx, maxy)` filter in - lon/lat; items whose `bbox` doesn't intersect are - dropped before hrefs are resolved. + bbox: M6 — **input filter**, `(minx, miny, maxx, maxy)` in + **lon/lat** (EPSG:4326). Selects *which STAC items* are read: + items whose footprint doesn't intersect it are dropped before + their hrefs are resolved. It does **not** clip the output — that + is `bounds`. (Note the difference from odc-stac, where `bbox` + sets the output extent.) max_items: M6 — cap the number of items consumed (after bbox filtering). Useful for quick-look workflows. signer: Optional signer (e.g. a @@ -1742,7 +1745,10 @@ def from_stac( exclusive with `crs`/`resolution`/`bounds`. crs: Target CRS for an explicit grid (with `resolution`+`bounds`). resolution: Target pixel size for an explicit grid. - bounds: Target `(minx, miny, maxx, maxy)` for an explicit grid. + bounds: **Output grid extent** — `(minx, miny, maxx, maxy)` in the + **target `crs`** (not lon/lat). Sets the extent every timestep is + warped/clipped to for the explicit grid. This is the output + window; to instead pre-filter *which items* are read, use `bbox`. anchor: Grid-snap rule for the explicit grid (`"edge"`). Returns: From 3213c9e2ba44a4c647e7ca2a1d471bfb1ce7db90 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 20:12:11 +0200 Subject: [PATCH 06/12] reformat --- src/pyramids/dataset/collection.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/pyramids/dataset/collection.py b/src/pyramids/dataset/collection.py index a8cdd43d81..d71ac1b1ba 100644 --- a/src/pyramids/dataset/collection.py +++ b/src/pyramids/dataset/collection.py @@ -2,7 +2,6 @@ from __future__ import annotations -import datetime as dt import fnmatch import numbers import re @@ -10,6 +9,7 @@ import textwrap import warnings from collections.abc import Callable, Sequence +from datetime import datetime from pathlib import Path from typing import TYPE_CHECKING, Any, cast @@ -1839,8 +1839,8 @@ def from_files( glob: str = _DEFAULT_GLOB, date_format: str | None = None, date_regex: str = r"\d{4}.\d{2}.\d{2}", - start: dt.datetime | None = None, - end: dt.datetime | None = None, + start: datetime | None = None, + end: datetime | None = None, meta: RasterMeta | None = None, gdal_env: dict[str, str] | None = None, validate: bool = False, @@ -1911,7 +1911,7 @@ def from_files( ``` """ resolved = cls._resolve_files(files, glob) - time_axis: list[dt.datetime] | None = None + time_axis: list[datetime] | None = None if date_format is not None: dates = [ cls._parse_date(Path(f).name, date_regex, date_format) for f in resolved @@ -1942,7 +1942,7 @@ def from_files( def _build( cls, files: list[str], - time_axis: list[dt.datetime] | list[int] | None, + time_axis: list[datetime] | list[int] | None, *, meta: RasterMeta | None, gdal_env: dict[str, str] | None, @@ -2004,12 +2004,12 @@ def _resolve_files( return resolved @staticmethod - def _parse_date(name: str, regex: str, fmt: str) -> dt.datetime: + def _parse_date(name: str, regex: str, fmt: str) -> datetime: """Return the date in ``name`` — the ``regex`` match parsed with ``fmt``.""" match = re.search(regex, name) if match is None: raise ValueError(f"date pattern {regex!r} matched no date in {name!r}") - return dt.datetime.strptime(match.group(), fmt) + return datetime.strptime(match.group(), fmt) @staticmethod def _parse_number(name: str, regex: str) -> int: @@ -2261,8 +2261,8 @@ def read_multiple_files( order = sorted(range(len(resolved)), key=dates.__getitem__) resolved = [resolved[i] for i in order] dates = [dates[i] for i in order] - start_dt = dt.datetime.strptime(start, fmt) if start is not None else None - end_dt = dt.datetime.strptime(end, fmt) if end is not None else None + start_dt = datetime.strptime(start, fmt) if start is not None else None + end_dt = datetime.strptime(end, fmt) if end is not None else None if start_dt is not None or end_dt is not None: kept = [ (f, d) From 83317586cb22ed25963a4989701911247de524b6 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 20:17:12 +0200 Subject: [PATCH 07/12] docs(how-to): add a pre-commit hooks guide Document the pre-commit setup: install, triggering (all files, a subset, a single hook by id), the full hook catalogue with speed tiers, and how to skip hooks via SKIP / --no-verify (with the CI skip set). Add it to the How-to nav. --- docs/how-to/pre-commit-hooks.md | 154 ++++++++++++++++++++++++++++++++ mkdocs.yml | 1 + 2 files changed, 155 insertions(+) create mode 100644 docs/how-to/pre-commit-hooks.md diff --git a/docs/how-to/pre-commit-hooks.md b/docs/how-to/pre-commit-hooks.md new file mode 100644 index 0000000000..24407a6961 --- /dev/null +++ b/docs/how-to/pre-commit-hooks.md @@ -0,0 +1,154 @@ +# Pre-commit hooks + +This page explains the pyramids [pre-commit](https://pre-commit.com/) setup: what the hooks are, how to install them, +how to trigger them (all, a subset, or one), and how to skip the slow ones. The hook definitions live in +[`.pre-commit-config.yaml`](../../.pre-commit-config.yaml); the fast checks run in isolated tool environments while a few +heavier hooks (`mypy`, `pytest-check`, `doctest`, `notebook-check`, `pixi-lock-check`) shell out to the pixi **dev** +environment. + +The same config is the local mirror of CI: the `pre-commit` job in +[`.github/workflows/lint.yml`](../../.github/workflows/lint.yml) runs `pre-commit run --all-files` with the slow test +hooks skipped, so getting a clean local run is the fastest way to keep that job green. + +## Install (one-time) + +Register the git hook so it fires on every `git commit`: + +```bash +pixi run --frozen -e dev pre-commit install +``` + +`--frozen` makes pixi use `pixi.lock` as-is instead of re-solving every platform on each run (which is slow and can hang +on Windows). The first hook run also builds an isolated toolchain per hook repo (a few minutes, needs network) — that is +expected, not a failure. + +## Trigger the hooks + +### Automatically, on commit + +Once installed, the hooks run against the **staged files** every time you commit: + +```bash +git commit -m "feat(dataset): ..." +``` + +`fail_fast: true` is set, so the first hook that modifies a file (e.g. `ruff-format`) stops the run — re-stage and commit +again until it passes. + +### Manually, on the whole tree + +Run every hook against all files without committing (this is what CI does): + +```bash +pixi run --frozen -e dev pre-commit run --all-files +``` + +### On specific files only + +```bash +pixi run --frozen -e dev pre-commit run --files src/pyramids/dataset/collection.py tests/dataset/collection/test_meta.py +``` + +To scope the run to just what your branch changed: + +```bash +pixi run --frozen -e dev pre-commit run --files $(git diff --name-only origin/main...HEAD) +``` + +### A single specific hook + +Pass the hook **id** (the left column in the table below). Combine with `--all-files` or `--files` to choose the scope: + +```bash +pixi run --frozen -e dev pre-commit run ruff-check --all-files # lint only +pixi run --frozen -e dev pre-commit run ruff-format --all-files # format only +pixi run --frozen -e dev pre-commit run mypy --all-files # type-check only +pixi run --frozen -e dev pre-commit run pytest-check --all-files # the full test suite +``` + +## The hooks + +| id | What it does | Speed | +| --- | --- | --- | +| `check-toml` / `check-json` / `check-yaml` | Validate config file syntax | fast | +| `end-of-file-fixer` / `trailing-whitespace` / `mixed-line-ending` | Whitespace / newline normalisation | fast | +| `pretty-format-json` / `requirements-txt-fixer` | Canonicalise JSON and `requirements.txt` | fast | +| `check-added-large-files` | Block files over 2 MB | fast | +| `check-merge-conflict` / `detect-private-key` / `debug-statements` | Catch conflict markers, keys, stray `breakpoint()` | fast | +| `no-commit-to-branch` | Refuse a commit made directly on `main` | fast | +| `ruff-check` | Lint Python + notebooks (auto-fix) | fast | +| `ruff-format` | Format Python + notebooks | fast | +| `nbstripout` | Strip notebook outputs / execution counts | fast | +| `beautysh` / `shellcheck` | Format and lint shell scripts | fast | +| `check-summary-*` / `check-description-*` / `check-second-line-empty` | Commit-message conventional-commit checks | fast | +| `bandit` | Python security linter | medium | +| `gitleaks` | Scan staged changes for secrets | medium | +| `checkov` | Infrastructure-as-code security scan | medium | +| `mypy` | Static type-check (in the pixi `dev` env) | **slow** | +| `pytest-check` | Full test suite with coverage (`-m "not plot"`) | **slow** | +| `doctest` | `--doctest-modules src` under the Agg backend | **slow** | +| `notebook-check` | `nbval` execution of the example notebooks | **slow** | +| `pixi-lock-check` | Fail if `pixi.lock` is stale vs `pyproject.toml` | medium | + +## Skip hooks + +### Skip specific hooks with `SKIP` + +pre-commit reads the `SKIP` environment variable (a comma-separated list of hook ids) at commit time and at +`pre-commit run` time. Everything else still runs. To skip the slow test trio on a commit: + +```bash +# Git Bash — inline, scoped to the single command: +SKIP=pytest-check,doctest,notebook-check git commit -m "docs: ..." +``` + +```powershell +# PowerShell — persists for the rest of the session (clear with: Remove-Item Env:SKIP): +$env:SKIP = 'pytest-check,doctest,notebook-check' +git commit -m "docs: ..." +``` + +To make the skip permanent (also applies to commits from the PyCharm/JetBrains commit dialog, which inherits the +environment it was launched with): + +```powershell +[Environment]::SetEnvironmentVariable('SKIP', 'pytest-check,doctest,notebook-check', 'User') +``` + +Restart your shells / IDE afterward. Note that a permanent `SKIP` also excludes those hooks from +`pre-commit run --all-files`, so they never run locally until you clear it — fine if you rely on CI as the backstop. + +### The CI skip set + +The `lint.yml` job runs everything **except** the hooks it can't or shouldn't run there. To reproduce that job exactly: + +```bash +SKIP=no-commit-to-branch,gitleaks,pytest-check,doctest,notebook-check,pixi-lock-check \ + pixi run --frozen -e dev pre-commit run --all-files +``` + +### Skip every hook + +`--no-verify` bypasses **all** hooks for a single commit — use it sparingly, since it also skips the fast formatting and +lint that keep the diff clean: + +```bash +git commit --no-verify -m "wip" +``` + +## Gotchas + +- **pixi startup cost.** `pixi run` has a noticeable cold-start on Windows (~15–25 s). That is env warm-up, not a hung + hook. +- **`--frozen` always.** Without it, pixi re-solves every platform on each hook invocation, which is slow and can hang. +- **`-p no:cacheprovider`.** The pytest-based hooks disable the cache because writing `.pytest_cache` fails with + `WinError 183` on the Google-Drive-synced tree. +- **Agg backend for doctests.** The `doctest` hook forces `matplotlib.use("Agg")` so no plotting doctest pops a GUI + window that would block the commit. +- **`no-commit-to-branch` blocks `main`.** Commits must be made on a feature branch; this hook refuses a direct commit on + `main`. + +## See also + +- [Testing & CI](testing.md) — how the test suite is sliced across CI jobs and reproduced locally. +- [`.pre-commit-config.yaml`](../../.pre-commit-config.yaml) — the source of truth for every hook. diff --git a/mkdocs.yml b/mkdocs.yml index 5cacf18b38..209520c9b5 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -256,6 +256,7 @@ nav: - Dev Container: how-to/devcontainer.md - Docker & GHCR: how-to/docker.md - Parallel windowed reads: how-to/parallel-reads.md + - Pre-commit hooks: how-to/pre-commit-hooks.md - Recipes: how-to/recipes.md - Testing & CI: - Overview: how-to/testing.md From 850edd301eb930c9a1e9b5c71822e2c92543f3e6 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 21:01:32 +0200 Subject: [PATCH 08/12] refactor(collection)!: group from_stac target-grid params into a Grid dataclass Collapse from_stac's five target-grid keyword arguments (like, crs, resolution, bounds, anchor) into a single grid: Grid | None parameter. Grid is a frozen dataclass whose __post_init__ enforces the mode invariants (like xor the crs/resolution/bounds trio, the trio being all-or-nothing, and the anchor value) previously checked inside _resolve_target_grid. This drops from_stac from 14 to 10 parameters and mirrors odc.stac.load's geobox= grouping. Grid is exported from pyramids.dataset; _resolve_target_grid now takes a Grid and only builds the template. BREAKING CHANGE: DatasetCollection.from_stac no longer accepts like/crs/resolution/bounds/anchor keyword arguments; pass grid=Grid(...) instead, e.g. grid=Grid(like=ds) or grid=Grid(crs=32633, resolution=10, bounds=(...)). --- docs/reference/dataset/collection.md | 2 +- docs/tutorials/stac.md | 15 ++-- src/pyramids/dataset/__init__.py | 2 + src/pyramids/dataset/_stac.py | 78 ++++++---------- src/pyramids/dataset/collection.py | 37 +++----- src/pyramids/dataset/grid.py | 104 ++++++++++++++++++++++ tests/dataset/stac/test_from_stac_grid.py | 100 +++++++++++++-------- 7 files changed, 217 insertions(+), 121 deletions(-) create mode 100644 src/pyramids/dataset/grid.py diff --git a/docs/reference/dataset/collection.md b/docs/reference/dataset/collection.md index 96dbd1aba5..406033c1de 100644 --- a/docs/reference/dataset/collection.md +++ b/docs/reference/dataset/collection.md @@ -29,7 +29,7 @@ classDiagram class DatasetCollection { +from_dataset(dataset, time_length) DatasetCollection +from_files(files, glob, date_format, date_regex, start, end, meta, gdal_env, validate) DatasetCollection - +from_stac(items, asset, patch_url, bbox, max_items, signer, align, skip_missing, groupby, like, crs, resolution, bounds, anchor) DatasetCollection + +from_stac(items, asset, patch_url, bbox, max_items, signer, align, skip_missing, groupby, grid) DatasetCollection +from_point(lat, lon, collection, bands, start_date, end_date, edge_size, resolution, units, stac, query, signer, align) DatasetCollection +from_zarr(store, storage_options) DatasetCollection +from_archive(url_or_path, kind, member_glob, meta) DatasetCollection diff --git a/docs/tutorials/stac.md b/docs/tutorials/stac.md index 99edc3f34d..799349d9b5 100644 --- a/docs/tutorials/stac.md +++ b/docs/tutorials/stac.md @@ -87,18 +87,21 @@ into one timestep, mosaicked first-valid: daily = DatasetCollection.from_stac(items, asset="visual", groupby="solar_day") ``` -### Match a target grid — `like=` / `crs`+`resolution`+`bounds` +### Match a target grid — `grid=Grid(...)` To guarantee pixel co-registration, resample every timestep onto an explicit -grid — either an existing `Dataset` (`like=`) or a CRS + resolution + bounds -(snapped to the resolution so independently-built grids align): +grid with the `grid` parameter and a `Grid` — either an existing `Dataset` +(`Grid(like=...)`) or a CRS + resolution + bounds (snapped to the resolution so +independently-built grids align): ```python -cube = DatasetCollection.from_stac(items, asset="B04", like=reference_dataset) +from pyramids.dataset import Grid + +cube = DatasetCollection.from_stac(items, asset="B04", grid=Grid(like=reference_dataset)) cube = DatasetCollection.from_stac( - items, asset="B04", crs=32633, resolution=10, - bounds=(600000, 5300000, 610000, 5310000), + items, asset="B04", + grid=Grid(crs=32633, resolution=10, bounds=(600000, 5300000, 610000, 5310000)), ) ``` diff --git a/src/pyramids/dataset/__init__.py b/src/pyramids/dataset/__init__.py index ac9b12947a..a8808226f9 100644 --- a/src/pyramids/dataset/__init__.py +++ b/src/pyramids/dataset/__init__.py @@ -5,6 +5,7 @@ from pyramids.dataset.abstract_dataset import DEFAULT_NO_DATA_VALUE from pyramids.dataset.collection import DatasetCollection from pyramids.dataset.dataset import Dataset, NoDataSentinelWarning +from pyramids.dataset.grid import Grid from pyramids.dataset.transform import GeoTransform from pyramids.dataset.window import Window @@ -13,6 +14,7 @@ "DatasetCollection", "DEFAULT_NO_DATA_VALUE", "GeoTransform", + "Grid", "GroundControlPoint", "NoDataSentinelWarning", "RasterMeta", diff --git a/src/pyramids/dataset/_stac.py b/src/pyramids/dataset/_stac.py index fe742a288f..a3973c3581 100644 --- a/src/pyramids/dataset/_stac.py +++ b/src/pyramids/dataset/_stac.py @@ -33,6 +33,7 @@ if TYPE_CHECKING: from pyramids.dataset.collection import DatasetCollection + from pyramids.dataset.grid import Grid def _iter_items(items: Any) -> list[Any]: @@ -195,11 +196,7 @@ def from_stac( align: bool = True, skip_missing: bool = False, groupby: str | None = None, - like: Any = None, - crs: int | str | None = None, - resolution: float | None = None, - bounds: Sequence[float] | None = None, - anchor: str = "edge", + grid: Grid | None = None, ) -> DatasetCollection: """Build a :class:`DatasetCollection` from a STAC ItemCollection. @@ -288,19 +285,13 @@ def from_stac( `merge_rasters(method="first")` (first-valid pixel wins on overlap; see :func:`_from_stac_solar_day`). `time_length` is the number of distinct solar days, in chronological order. Single-asset only. - like: Optional target grid as an existing - :class:`~pyramids.dataset.Dataset`; every timestep of the built - cube is reprojected/resampled onto its CRS + grid (via - :meth:`DatasetCollection.align`), guaranteeing pixel co-registration. - Mutually exclusive with `crs`/`resolution`/`bounds`. - crs: Target CRS (EPSG int or CRS string) for an explicit target grid. - Must be given together with `resolution` and `bounds`. - resolution: Target pixel size (CRS units) for an explicit target grid. - bounds: Target `(minx, miny, maxx, maxy)` extent (in `crs`) for an - explicit target grid. - anchor: Grid-snap rule for the explicit `crs`/`resolution`/`bounds` - grid. `"edge"` (default) snaps pixel edges to multiples of - `resolution` (so independently-built grids co-register). + grid: Optional :class:`~pyramids.dataset.Grid` describing the target + output grid; every timestep of the built cube is reprojected / + resampled onto it (via :meth:`DatasetCollection.align`), guaranteeing + pixel co-registration. `None` (default) or an empty `Grid()` keeps + each timestep's native grid. Use `Grid(like=)` to match an + existing grid, or `Grid(crs=..., resolution=..., bounds=...)` for an + explicit one. Returns: DatasetCollection: A file-backed collection whose `time_length` @@ -352,7 +343,7 @@ def _sign(href: str) -> str: # pyramids.dataset import cycle (see _resolve_asset_href above). from pyramids.dataset.collection import DatasetCollection - target_grid = _resolve_target_grid(like, crs, resolution, bounds, anchor) + target_grid = _resolve_target_grid(grid) if groupby is not None: if groupby != "solar_day": @@ -386,46 +377,27 @@ def _sign(href: str) -> str: return collection -def _resolve_target_grid( - like: Any, - crs: int | str | None, - resolution: float | None, - bounds: Sequence[float] | None, - anchor: str, -) -> Any: - """Resolve the PC-2 grid-match arguments to a template Dataset (or None). +def _resolve_target_grid(grid: Grid | None) -> Any: + """Resolve a :class:`~pyramids.dataset.Grid` to a template Dataset (or None). + + The mode invariants (``like`` xor the ``crs``/``resolution``/``bounds`` trio, + the trio being all-or-nothing, and the ``anchor`` value) are validated by + :meth:`Grid.__post_init__`, so this only has to build the template. Args: - like: An existing :class:`~pyramids.dataset.Dataset` to match, or - `None`. - crs: Target CRS (with `resolution` + `bounds`) for an explicit grid. - resolution: Target pixel size. - bounds: Target `(minx, miny, maxx, maxy)` extent. - anchor: Grid-snap rule (`"edge"` supported). + grid: A :class:`~pyramids.dataset.Grid`, or `None`. Returns: - The `like` Dataset, a freshly built template Dataset for an explicit - grid, or `None` when no grid-match was requested. - - Raises: - ValueError: `like` is combined with `crs`/`resolution`/`bounds`; the - explicit-grid trio is given only partially; or `anchor` is - unsupported. + The `grid.like` Dataset, a freshly built template Dataset for an explicit + grid, or `None` when no grid was requested (``None`` or an empty + ``Grid()``). """ - explicit = (crs, resolution, bounds) - if like is not None: - if any(v is not None for v in explicit): - raise ValueError("like= is mutually exclusive with crs/resolution/bounds.") - return like - if all(v is None for v in explicit): + if grid is None or grid.is_empty: return None - if any(v is None for v in explicit): - raise ValueError( - "crs, resolution, and bounds must all be given together (or use like=)." - ) - if anchor != "edge": - raise ValueError(f"anchor must be 'edge', got {anchor!r}.") - # The two guards above already proved none of the trio is None. + if grid.like is not None: + return grid.like + # The trio is complete here (guaranteed by Grid.__post_init__). + crs, resolution, bounds = grid.crs, grid.resolution, grid.bounds assert crs is not None and resolution is not None and bounds is not None import math diff --git a/src/pyramids/dataset/collection.py b/src/pyramids/dataset/collection.py index d71ac1b1ba..b0b2fec0b9 100644 --- a/src/pyramids/dataset/collection.py +++ b/src/pyramids/dataset/collection.py @@ -36,6 +36,7 @@ from pyramids.dataset._stac import from_stac as _from_stac from pyramids.dataset.abstract_dataset import CATALOG from pyramids.dataset.dataset import Dataset +from pyramids.dataset.grid import Grid from pyramids.dataset.merge import merge_rasters from pyramids.dataset.ops._geobox_zarr import ( ZARR_SCHEMA_VERSION, @@ -1672,11 +1673,7 @@ def from_stac( align: bool = True, skip_missing: bool = False, groupby: str | None = None, - like: Any = None, - crs: int | str | None = None, - resolution: float | None = None, - bounds=None, - anchor: str = "edge", + grid: Grid | None = None, ) -> DatasetCollection: """Build a collection from a STAC ItemCollection. @@ -1699,8 +1696,9 @@ def from_stac( **lon/lat** (EPSG:4326). Selects *which STAC items* are read: items whose footprint doesn't intersect it are dropped before their hrefs are resolved. It does **not** clip the output — that - is `bounds`. (Note the difference from odc-stac, where `bbox` - sets the output extent.) + is the `grid`'s bounds (see :class:`~pyramids.dataset.Grid`). + (Note the difference from odc-stac, where `bbox` sets the output + extent.) max_items: M6 — cap the number of items consumed (after bbox filtering). Useful for quick-look workflows. signer: Optional signer (e.g. a @@ -1740,20 +1738,17 @@ def from_stac( stack from tiled imagery over an AOI that spans several tiles. Do **not** use it for non-overpass data (climate model output, already-mosaicked products) — there `groupby=None` is correct. - like: Optional target-grid :class:`~pyramids.dataset.Dataset`; - every timestep is aligned onto its CRS + grid. Mutually - exclusive with `crs`/`resolution`/`bounds`. - crs: Target CRS for an explicit grid (with `resolution`+`bounds`). - resolution: Target pixel size for an explicit grid. - bounds: **Output grid extent** — `(minx, miny, maxx, maxy)` in the - **target `crs`** (not lon/lat). Sets the extent every timestep is - warped/clipped to for the explicit grid. This is the output - window; to instead pre-filter *which items* are read, use `bbox`. - anchor: Grid-snap rule for the explicit grid (`"edge"`). + grid: Optional :class:`~pyramids.dataset.Grid` describing the target + **output grid** every timestep is warped/aligned onto. `None` + (default) or an empty `Grid()` keeps each timestep's native grid. + Use `Grid(like=)` to match an existing grid, or + `Grid(crs=..., resolution=..., bounds=...)` for an explicit one + (its `bounds` are the output window, in the target CRS — distinct + from `bbox`, which filters input items in lon/lat). Returns: DatasetCollection: File-backed collection (or grid-aligned - collection when `like`/`crs` is given). + collection when a non-empty `grid` is given). """ return _from_stac( items, @@ -1765,11 +1760,7 @@ def from_stac( align=align, skip_missing=skip_missing, groupby=groupby, - like=like, - crs=crs, - resolution=resolution, - bounds=bounds, - anchor=anchor, + grid=grid, ) @classmethod diff --git a/src/pyramids/dataset/grid.py b/src/pyramids/dataset/grid.py new file mode 100644 index 0000000000..f98dd2919a --- /dev/null +++ b/src/pyramids/dataset/grid.py @@ -0,0 +1,104 @@ +"""The :class:`Grid` target-grid specification for collection constructors.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any + + +@dataclass(frozen=True, slots=True) +class Grid: + """Target output grid for a reprojected / aligned collection. + + Describes the grid every timestep is warped onto — currently by + :meth:`~pyramids.dataset.DatasetCollection.from_stac`. It has three states: + + * **empty** (every field left at its default) — no reprojection; each + timestep keeps its native grid. + * **template** (``like``) — match an existing + :class:`~pyramids.dataset.Dataset`'s CRS + geotransform + shape exactly. + * **explicit** (``crs`` + ``resolution`` + ``bounds``, all three) — build a + grid from those values, with the extent snapped by ``anchor``. + + ``like`` and the ``crs`` / ``resolution`` / ``bounds`` trio are mutually + exclusive, and the trio is all-or-nothing (there is no inference of a + missing member). These invariants are enforced at construction, so an + invalid combination raises here rather than deep inside a constructor. + + Args: + like: An existing :class:`~pyramids.dataset.Dataset` whose grid to copy, + or ``None``. + crs: Target CRS (EPSG int like ``32633``, or a CRS string) for an + explicit grid. + resolution: Target pixel size, in the target CRS's units. + bounds: Target ``(minx, miny, maxx, maxy)`` extent, expressed in ``crs`` + (not lon/lat). + anchor: Grid-snap rule for the explicit grid. Only ``"edge"`` is + supported today (pixel edges snap to multiples of ``resolution`` so + independently built grids co-register). + + Raises: + ValueError: ``like`` is combined with any of ``crs`` / ``resolution`` / + ``bounds``; the explicit trio is given only partially; or ``anchor`` + is not ``"edge"``. + + Examples: + - An explicit target grid: + + ```python + >>> from pyramids.dataset import Grid + >>> grid = Grid(crs=32633, resolution=10, bounds=(0, 0, 1000, 1000)) + >>> grid.is_empty + False + + ``` + - An empty grid means "keep the native grid": + + ```python + >>> from pyramids.dataset import Grid + >>> Grid().is_empty + True + + ``` + - Mixing the two modes is rejected: + + ```python + >>> from pyramids.dataset import Grid + >>> Grid(crs=32633, resolution=10) + Traceback (most recent call last): + ... + ValueError: Grid: crs, resolution, and bounds must all be given together (or use like=). + + ``` + """ + + like: Any = None + crs: int | str | None = None + resolution: float | None = None + bounds: tuple[float, float, float, float] | None = None + anchor: str = "edge" + + def __post_init__(self) -> None: + """Validate the mutually-exclusive modes and the anchor.""" + given = [v is not None for v in (self.crs, self.resolution, self.bounds)] + if self.like is not None and any(given): + raise ValueError( + "Grid: like= is mutually exclusive with crs/resolution/bounds." + ) + if self.like is None and any(given) and not all(given): + raise ValueError( + "Grid: crs, resolution, and bounds must all be given together " + "(or use like=)." + ) + if self.anchor != "edge": + raise ValueError(f"Grid: anchor must be 'edge', got {self.anchor!r}.") + + @property + def is_empty(self) -> bool: + """Whether no target grid is specified (the native grid is kept).""" + return ( + self.like is None + and self.crs is None + and self.resolution is None + and self.bounds is None + ) diff --git a/tests/dataset/stac/test_from_stac_grid.py b/tests/dataset/stac/test_from_stac_grid.py index 9695691aca..1742ea0dfd 100644 --- a/tests/dataset/stac/test_from_stac_grid.py +++ b/tests/dataset/stac/test_from_stac_grid.py @@ -1,11 +1,11 @@ -"""Tests for PC-2 grid match: from_stac(like=/crs=/resolution=/bounds=).""" +"""Tests for the grid match: from_stac(grid=Grid(like=.../crs=.../...)).""" from __future__ import annotations import numpy as np import pytest -from pyramids.dataset import Dataset, DatasetCollection +from pyramids.dataset import Dataset, DatasetCollection, Grid from pyramids.dataset._stac import _resolve_target_grid pytestmark = pytest.mark.core @@ -53,42 +53,73 @@ def template(tmp_path): ) -class TestResolveTargetGrid: - """Tests for the _resolve_target_grid helper.""" +class TestGrid: + """Tests for the Grid dataclass and its construction-time validation.""" - def test_none_when_nothing_requested(self): - """No grid args -> None (no alignment). + def test_empty_is_empty(self): + """A default Grid reports empty. Test scenario: - All grid params absent. + Grid() with no fields set -> is_empty True. """ - assert _resolve_target_grid(None, None, None, None, "edge") is None + assert Grid().is_empty is True, "default Grid should be empty" - def test_like_returned_directly(self, template): - """A like Dataset is returned as the template. + def test_explicit_not_empty(self): + """An explicit-grid Grid is not empty. Test scenario: - like= passes through unchanged. + crs + resolution + bounds -> is_empty False. """ - assert _resolve_target_grid(template, None, None, None, "edge") is template + grid = Grid(crs=4326, resolution=1.0, bounds=(0, 0, 4, 4)) + assert grid.is_empty is False, "explicit Grid should not be empty" def test_like_with_explicit_raises(self, template): - """like= combined with crs/resolution/bounds raises. + """like combined with crs/resolution/bounds raises. Test scenario: - Mutually-exclusive grid specs are rejected. + Mutually-exclusive grid specs are rejected at construction. """ with pytest.raises(ValueError, match="mutually exclusive"): - _resolve_target_grid(template, 4326, 1.0, (0, 0, 4, 4), "edge") + Grid(like=template, crs=4326, resolution=1.0, bounds=(0, 0, 4, 4)) def test_partial_explicit_raises(self): """An incomplete crs/resolution/bounds trio raises. Test scenario: - crs + resolution without bounds is rejected. + crs + resolution without bounds is rejected at construction. """ with pytest.raises(ValueError, match="all be given together"): - _resolve_target_grid(None, 4326, 1.0, None, "edge") + Grid(crs=4326, resolution=1.0) + + def test_unknown_anchor_raises(self): + """An unsupported anchor raises. + + Test scenario: + anchor='center' is not implemented. + """ + with pytest.raises(ValueError, match="anchor must be"): + Grid(crs=4326, resolution=1.0, bounds=(0, 0, 4, 4), anchor="center") + + +class TestResolveTargetGrid: + """Tests for the _resolve_target_grid helper (Grid -> template Dataset).""" + + def test_none_when_nothing_requested(self): + """None or an empty Grid -> None (no alignment). + + Test scenario: + No grid, and an empty Grid, both resolve to None. + """ + assert _resolve_target_grid(None) is None + assert _resolve_target_grid(Grid()) is None + + def test_like_returned_directly(self, template): + """A like Dataset is returned as the template. + + Test scenario: + Grid(like=...) passes the Dataset through unchanged. + """ + assert _resolve_target_grid(Grid(like=template)) is template def test_explicit_builds_snapped_template(self): """An explicit grid builds a template snapped to the resolution. @@ -96,31 +127,24 @@ def test_explicit_builds_snapped_template(self): Test scenario: bounds (0.4, 0.4, 3.6, 3.6) at resolution 1 snap to (0,0,4,4) -> 4x4. """ - tpl = _resolve_target_grid(None, 4326, 1.0, (0.4, 0.4, 3.6, 3.6), "edge") + tpl = _resolve_target_grid( + Grid(crs=4326, resolution=1.0, bounds=(0.4, 0.4, 3.6, 3.6)) + ) assert tpl.epsg == 4326, f"epsg: {tpl.epsg}" assert (tpl.rows, tpl.columns) == (4, 4), f"shape: {(tpl.rows, tpl.columns)}" - def test_unknown_anchor_raises(self): - """An unsupported anchor raises. - - Test scenario: - anchor='center' is not implemented. - """ - with pytest.raises(ValueError, match="anchor must be"): - _resolve_target_grid(None, 4326, 1.0, (0, 0, 4, 4), "center") - class TestFromStacGridMatch: - """Tests for from_stac grid match via like= / explicit grid.""" + """Tests for from_stac grid match via Grid(like=) / explicit Grid.""" def test_like_matches_grid(self, offset_grid_items, template): - """like= resamples every timestep onto the template's grid. + """grid=Grid(like=) resamples every timestep onto the template's grid. Test scenario: Coarse 2x2 items -> a 4x4 cube matching the template. """ coll = DatasetCollection.from_stac( - offset_grid_items, asset="data", like=template + offset_grid_items, asset="data", grid=Grid(like=template) ) assert coll.time_length == 2, f"expected 2 timesteps, got {coll.time_length}" first = coll.datasets[0] @@ -131,7 +155,7 @@ def test_like_matches_grid(self, offset_grid_items, template): assert first.epsg == 4326, f"epsg: {first.epsg}" def test_explicit_grid_matches(self, offset_grid_items): - """crs/resolution/bounds build and match an explicit target grid. + """grid=Grid(crs/resolution/bounds) builds and matches an explicit grid. Test scenario: A 1-degree grid over (0,0,4,4) yields 4x4 timesteps. @@ -139,9 +163,7 @@ def test_explicit_grid_matches(self, offset_grid_items): coll = DatasetCollection.from_stac( offset_grid_items, asset="data", - crs=4326, - resolution=1.0, - bounds=(0.0, 0.0, 4.0, 4.0), + grid=Grid(crs=4326, resolution=1.0, bounds=(0.0, 0.0, 4.0, 4.0)), ) first = coll.datasets[0] assert (first.rows, first.columns) == ( @@ -150,10 +172,10 @@ def test_explicit_grid_matches(self, offset_grid_items): ), f"shape: {(first.rows, first.columns)}" def test_no_grid_match_keeps_native(self, offset_grid_items): - """Without like/crs the cube keeps the native (coarse) grid. + """Without a grid the cube keeps the native (coarse) grid. Test scenario: - No grid args -> 2x2 native timesteps (back-compat). + No grid arg -> 2x2 native timesteps. """ coll = DatasetCollection.from_stac(offset_grid_items, asset="data") first = coll.datasets[0] @@ -175,7 +197,9 @@ def test_oversize_grid_raises(self): """ with pytest.raises(ValueError, match="exceeding the"): # ~ (100 deg / 1e-4) ^2 pixels = 1e12, far over the 250M limit. - _resolve_target_grid(None, 4326, 0.0001, (0.0, 0.0, 100.0, 100.0), "edge") + _resolve_target_grid( + Grid(crs=4326, resolution=0.0001, bounds=(0.0, 0.0, 100.0, 100.0)) + ) def test_just_under_limit_builds(self): """A large-but-allowed grid still builds (S2-tile-scale). @@ -184,7 +208,7 @@ def test_just_under_limit_builds(self): A 5000x5000 grid (25M px) is well under the 250M ceiling. """ tpl = _resolve_target_grid( - None, 32633, 10.0, (0.0, 0.0, 50000.0, 50000.0), "edge" + Grid(crs=32633, resolution=10.0, bounds=(0.0, 0.0, 50000.0, 50000.0)) ) assert (tpl.rows, tpl.columns) == ( 5000, From 1755eba8ec2c580faa532b4643107f6ab5222ffe Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 21:12:22 +0200 Subject: [PATCH 09/12] feat(collection): resample from_point cubes onto the exact local-UTM grid Extend the Grid mechanism to from_point: it now builds a Grid(crs=, resolution, bounds=) and forwards it to from_stac, so every timestep is resampled and co-registered onto the exact edge_size x edge_size local-UTM grid the AOI defines. Previously the cube was left on the matched assets' native grid -- the documented 'not yet' PC-2 gap. _point_aoi_bbox now also returns the UTM AOI square it already computed (and discarded); from_point uses it as the Grid bounds. Tests assert the forwarded Grid carries the local UTM crs/resolution/bounds. --- src/pyramids/dataset/_stac.py | 39 ++++++++++++-------- src/pyramids/dataset/collection.py | 7 ++-- tests/dataset/collection/test_from_point.py | 40 +++++++++++++++++---- 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/pyramids/dataset/_stac.py b/src/pyramids/dataset/_stac.py index a3973c3581..d187eacb0b 100644 --- a/src/pyramids/dataset/_stac.py +++ b/src/pyramids/dataset/_stac.py @@ -29,11 +29,11 @@ from pyramids.base._artifacts import artifact_dir from pyramids.base._errors import StacAssetError +from pyramids.dataset.grid import Grid from pyramids.utm import utm_epsg if TYPE_CHECKING: from pyramids.dataset.collection import DatasetCollection - from pyramids.dataset.grid import Grid def _iter_items(items: Any) -> list[Any]: @@ -679,8 +679,10 @@ def _point_aoi_bbox( edge_size: int, resolution: float, units: str, -) -> tuple[int, tuple[float, float, float, float]]: - """Compute the local-UTM EPSG and the 4326 search bbox for a point cube. +) -> tuple[ + int, tuple[float, float, float, float], tuple[float, float, float, float] +]: + """Compute the local-UTM EPSG, the UTM AOI, and the 4326 search bbox. The center `(lat, lon)` is reprojected to its local UTM, snapped to the `resolution` grid, and expanded to a square AOI of `edge_size` pixels @@ -696,7 +698,10 @@ def _point_aoi_bbox( units: `"px"` or `"m"`. Returns: - A `(utm_epsg, bbox_4326)` tuple, with `bbox_4326` = `(w, s, e, n)`. + A `(utm_epsg, utm_bbox, bbox_4326)` tuple: the local UTM EPSG code, the + resolution-snapped AOI square `(minx, miny, maxx, maxy)` in that UTM CRS + (the exact target grid), and the same square reprojected to EPSG:4326 + `(w, s, e, n)` for the STAC search. Raises: ValueError: When `units` is not `"px"` or `"m"`. @@ -719,7 +724,7 @@ def _point_aoi_bbox( clon, clat = to_wgs.transform(x, y) lons.append(clon) lats.append(clat) - return epsg, (min(lons), min(lats), max(lons), max(lats)) + return epsg, utm_bbox, (min(lons), min(lats), max(lons), max(lats)) def from_point( @@ -746,12 +751,12 @@ def from_point( `resolution` grid, and expanded to a square AOI of `edge_size` pixels (or metres); that AOI (reprojected to EPSG:4326) drives the STAC search. - .. note:: - The returned cube is on the matched assets' native grid clipped to the - AOI — it is **not yet** resampled to an exact `edge_size`×`edge_size` - local-UTM grid. Exact target-grid resampling arrives with the - `geobox=`/`like=` grid match (PC-2). For now `from_point` is the - convenience AOI + search + stack wrapper. + The returned cube is resampled onto the exact `edge_size`×`edge_size` + local-UTM target grid the AOI defines: `from_point` builds a + :class:`~pyramids.dataset.Grid` (`crs` = the local UTM zone, `resolution`, + `bounds` = the snapped UTM square) and forwards it to :func:`from_stac`, so + every timestep is co-registered on that grid regardless of the assets' + native CRS. Args: lat: Center latitude in degrees (EPSG:4326). @@ -773,7 +778,8 @@ def from_point( align: Multi-asset resolution policy, forwarded to :func:`from_stac`. Returns: - DatasetCollection: A time-stacked cube over the point AOI. + DatasetCollection: A time-stacked cube over the point AOI, resampled + onto the exact `edge_size`×`edge_size` local-UTM grid. Raises: ValueError: When `units` is invalid, or the search yields no items. @@ -797,7 +803,9 @@ def from_point( ``` """ - _utm_epsg_code, bbox_4326 = _point_aoi_bbox(lat, lon, edge_size, resolution, units) + utm_epsg, utm_bbox, bbox_4326 = _point_aoi_bbox( + lat, lon, edge_size, resolution, units + ) from pyramids.stac.search import search @@ -809,7 +817,10 @@ def from_point( query=query, signer=signer, ) - return from_stac(items, bands, signer=signer, align=align) + # Resample every timestep onto the exact edge_size x edge_size local-UTM + # target grid the AOI defines (PC-2), so the point cube is co-registered. + grid = Grid(crs=utm_epsg, resolution=resolution, bounds=utm_bbox) + return from_stac(items, bands, signer=signer, align=align, grid=grid) def _bbox_ring(bbox: Sequence[float]) -> dict[str, Any]: diff --git a/src/pyramids/dataset/collection.py b/src/pyramids/dataset/collection.py index b0b2fec0b9..c8b269cdbb 100644 --- a/src/pyramids/dataset/collection.py +++ b/src/pyramids/dataset/collection.py @@ -1786,7 +1786,9 @@ def from_point( Thin forwarder to :func:`pyramids.dataset._stac.from_point`: reprojects `(lat, lon)` to its local UTM, snaps to the `resolution` grid, expands to an `edge_size`-pixel (or -metre) square AOI, searches `collection` over - that AOI + date range, and stacks the `bands` via :meth:`from_stac`. + that AOI + date range, and stacks the `bands` via :meth:`from_stac` — + resampling every timestep onto that exact local-UTM grid (through an + internally built :class:`~pyramids.dataset.Grid`). Args: lat: Center latitude in degrees (EPSG:4326). @@ -1804,7 +1806,8 @@ def from_point( align: Multi-asset resolution policy (see :meth:`from_stac`). Returns: - DatasetCollection: A time-stacked cube over the point AOI. + DatasetCollection: A time-stacked cube over the point AOI, on the + exact `edge_size`×`edge_size` local-UTM grid. """ kwargs: dict[str, Any] = { "collection": collection, diff --git a/tests/dataset/collection/test_from_point.py b/tests/dataset/collection/test_from_point.py index 78342ba30c..190b96eb2f 100644 --- a/tests/dataset/collection/test_from_point.py +++ b/tests/dataset/collection/test_from_point.py @@ -7,7 +7,7 @@ import pytest import pyramids.stac.search # noqa: F401 (ensure the submodule is in sys.modules) -from pyramids.dataset import DatasetCollection +from pyramids.dataset import DatasetCollection, Grid from pyramids.dataset._stac import _point_aoi_bbox, _utm_epsg, from_point pytestmark = pytest.mark.core @@ -54,7 +54,9 @@ def test_local_utm_selected(self): Test scenario: (46N, 11E) selects EPSG:32632. """ - epsg, _ = _point_aoi_bbox(46.0, 11.0, edge_size=64, resolution=10.0, units="px") + epsg, _, _ = _point_aoi_bbox( + 46.0, 11.0, edge_size=64, resolution=10.0, units="px" + ) assert epsg == 32632, f"expected UTM 32632, got {epsg}" def test_bbox_brackets_the_point(self): @@ -63,7 +65,7 @@ def test_bbox_brackets_the_point(self): Test scenario: (46N, 11E) lies within the returned (w, s, e, n). """ - _, (w, s, e, n) = _point_aoi_bbox( + _, _, (w, s, e, n) = _point_aoi_bbox( 46.0, 11.0, edge_size=64, resolution=10.0, units="px" ) assert w < 11.0 < e and s < 46.0 < n, f"point not bracketed by {(w, s, e, n)}" @@ -77,7 +79,7 @@ def test_edge_size_px_extent(self): """ import math - _, (w, _, e, _) = _point_aoi_bbox( + _, _, (w, _, e, _) = _point_aoi_bbox( 46.0, 11.0, edge_size=64, resolution=10.0, units="px" ) expected_deg = 640.0 / (111_320.0 * math.cos(math.radians(46.0))) @@ -91,16 +93,29 @@ def test_units_metres(self): Test scenario: 1000 m square is wider than a 64 px * 10 m = 640 m square. """ - _, m_bbox = _point_aoi_bbox( + _, _, m_bbox = _point_aoi_bbox( 0.0, 0.0, edge_size=1000, resolution=10.0, units="m" ) - _, px_bbox = _point_aoi_bbox( + _, _, px_bbox = _point_aoi_bbox( 0.0, 0.0, edge_size=64, resolution=10.0, units="px" ) assert (m_bbox[2] - m_bbox[0]) > (px_bbox[2] - px_bbox[0]), ( "metres AOI should be wider" ) + def test_utm_bbox_is_snapped_square(self): + """The UTM AOI (2nd return value) is a resolution-snapped square. + + Test scenario: + 64 px * 10 m -> a 640 m UTM square whose edges are multiples of 10. + """ + _, (minx, miny, maxx, maxy), _ = _point_aoi_bbox( + 46.0, 11.0, edge_size=64, resolution=10.0, units="px" + ) + assert (maxx - minx) == pytest.approx(640.0), f"width: {maxx - minx}" + assert (maxy - miny) == pytest.approx(640.0), f"height: {maxy - miny}" + assert minx % 10.0 == pytest.approx(0.0), f"minx not snapped: {minx}" + def test_invalid_units_raises(self): """An unsupported units value raises ValueError. @@ -133,6 +148,7 @@ def fake_search(stac, collection, *, bbox, datetime, query, signer): def fake_from_stac(items, asset, *, signer=None, align=True, **kw): captured["items"] = items captured["asset"] = asset + captured["grid"] = kw.get("grid") return "CUBE" monkeypatch.setattr(_SEARCH_MOD, "search", fake_search) @@ -167,6 +183,18 @@ def fake_from_stac(items, asset, *, signer=None, align=True, **kw): "B04", "B03", ], f"bands should pass through: {captured['asset']}" + assert isinstance(captured["grid"], Grid), ( + f"from_point should forward a Grid to from_stac: {captured['grid']!r}" + ) + assert captured["grid"].crs == 32632, ( + f"grid crs should be the local UTM zone: {captured['grid'].crs}" + ) + assert captured["grid"].resolution == 10.0, ( + f"grid resolution: {captured['grid'].resolution}" + ) + assert captured["grid"].bounds is not None, ( + "grid should carry the UTM AOI bounds" + ) def test_classmethod_forwards(self, monkeypatch): """DatasetCollection.from_point forwards to the _stac implementation. From 7d703734cb865cd7771b6e555ecb0563b8286d3b Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 21:31:29 +0200 Subject: [PATCH 10/12] test(grid): add a dedicated Grid unit test suite Add tests/dataset/test_grid.py covering Grid construction, the mode invariants (like xor the crs/resolution/bounds trio, the trio being all-or-nothing, and the anchor check), the is_empty property, frozen immutability, and equality -- every __post_init__ branch. Remove the now redundant TestGrid class from test_from_stac_grid.py (superseded by the dedicated module test), keeping its resolver and from_stac integration tests. --- tests/dataset/stac/test_from_stac_grid.py | 48 ------- tests/dataset/test_grid.py | 145 ++++++++++++++++++++++ 2 files changed, 145 insertions(+), 48 deletions(-) create mode 100644 tests/dataset/test_grid.py diff --git a/tests/dataset/stac/test_from_stac_grid.py b/tests/dataset/stac/test_from_stac_grid.py index 1742ea0dfd..16b69fba3f 100644 --- a/tests/dataset/stac/test_from_stac_grid.py +++ b/tests/dataset/stac/test_from_stac_grid.py @@ -53,54 +53,6 @@ def template(tmp_path): ) -class TestGrid: - """Tests for the Grid dataclass and its construction-time validation.""" - - def test_empty_is_empty(self): - """A default Grid reports empty. - - Test scenario: - Grid() with no fields set -> is_empty True. - """ - assert Grid().is_empty is True, "default Grid should be empty" - - def test_explicit_not_empty(self): - """An explicit-grid Grid is not empty. - - Test scenario: - crs + resolution + bounds -> is_empty False. - """ - grid = Grid(crs=4326, resolution=1.0, bounds=(0, 0, 4, 4)) - assert grid.is_empty is False, "explicit Grid should not be empty" - - def test_like_with_explicit_raises(self, template): - """like combined with crs/resolution/bounds raises. - - Test scenario: - Mutually-exclusive grid specs are rejected at construction. - """ - with pytest.raises(ValueError, match="mutually exclusive"): - Grid(like=template, crs=4326, resolution=1.0, bounds=(0, 0, 4, 4)) - - def test_partial_explicit_raises(self): - """An incomplete crs/resolution/bounds trio raises. - - Test scenario: - crs + resolution without bounds is rejected at construction. - """ - with pytest.raises(ValueError, match="all be given together"): - Grid(crs=4326, resolution=1.0) - - def test_unknown_anchor_raises(self): - """An unsupported anchor raises. - - Test scenario: - anchor='center' is not implemented. - """ - with pytest.raises(ValueError, match="anchor must be"): - Grid(crs=4326, resolution=1.0, bounds=(0, 0, 4, 4), anchor="center") - - class TestResolveTargetGrid: """Tests for the _resolve_target_grid helper (Grid -> template Dataset).""" diff --git a/tests/dataset/test_grid.py b/tests/dataset/test_grid.py new file mode 100644 index 0000000000..96ff80f7f4 --- /dev/null +++ b/tests/dataset/test_grid.py @@ -0,0 +1,145 @@ +"""Unit tests for the Grid target-grid dataclass.""" + +from __future__ import annotations + +import dataclasses + +import pytest + +from pyramids.dataset import Grid + +pytestmark = pytest.mark.core + +_LIKE = object() # Grid only checks `like is not None`; any sentinel works. +_BOUNDS = (0.0, 0.0, 10.0, 10.0) + + +class TestGrid: + """Tests for Grid construction, validation, and the is_empty property.""" + + def test_default_is_empty(self): + """A default Grid holds no fields and reports empty. + + Test scenario: + Grid() -> every grid field None, is_empty True. + """ + grid = Grid() + assert grid.like is None, f"like should default None, got {grid.like!r}" + assert grid.crs is None, f"crs should default None, got {grid.crs!r}" + assert grid.resolution is None, f"resolution default: {grid.resolution!r}" + assert grid.bounds is None, f"bounds should default None, got {grid.bounds!r}" + assert grid.anchor == "edge", f"anchor should default 'edge', got {grid.anchor!r}" + assert grid.is_empty is True, "default Grid should be empty" + + def test_like_only_valid_and_not_empty(self): + """A template Grid carries `like` and is not empty. + + Test scenario: + Grid(like=) -> is_empty False, like preserved. + """ + grid = Grid(like=_LIKE) + assert grid.like is _LIKE, "like should be stored unchanged" + assert grid.is_empty is False, "a like Grid is not empty" + + def test_explicit_trio_valid_and_not_empty(self): + """A complete explicit trio builds a valid, non-empty Grid. + + Test scenario: + Grid(crs, resolution, bounds) -> fields preserved, is_empty False. + """ + grid = Grid(crs=32633, resolution=10.0, bounds=_BOUNDS) + assert grid.crs == 32633, f"crs: {grid.crs}" + assert grid.resolution == 10.0, f"resolution: {grid.resolution}" + assert grid.bounds == _BOUNDS, f"bounds: {grid.bounds}" + assert grid.is_empty is False, "an explicit Grid is not empty" + + def test_crs_accepts_string(self): + """crs may be a CRS string, not only an EPSG int. + + Test scenario: + Grid(crs="EPSG:4326", resolution, bounds) constructs. + """ + grid = Grid(crs="EPSG:4326", resolution=0.5, bounds=_BOUNDS) + assert grid.crs == "EPSG:4326", f"crs string not preserved: {grid.crs}" + + @pytest.mark.parametrize( + "kwargs", + [ + {"crs": 4326}, + {"resolution": 10.0}, + {"bounds": _BOUNDS}, + ], + ) + def test_like_with_any_explicit_member_raises(self, kwargs): + """like combined with any explicit-grid member is rejected. + + Args: + kwargs: A single explicit-grid field to pair with `like`. + + Test scenario: + Grid(like=..., ) -> ValueError. + """ + with pytest.raises(ValueError, match="mutually exclusive"): + Grid(like=_LIKE, **kwargs) + + @pytest.mark.parametrize( + "kwargs", + [ + {"crs": 4326}, + {"resolution": 10.0}, + {"bounds": _BOUNDS}, + {"crs": 4326, "resolution": 10.0}, + {"crs": 4326, "bounds": _BOUNDS}, + {"resolution": 10.0, "bounds": _BOUNDS}, + ], + ) + def test_partial_trio_raises(self, kwargs): + """Any incomplete crs/resolution/bounds trio is rejected. + + Args: + kwargs: A strict subset of the explicit-grid trio. + + Test scenario: + Grid(<1 or 2 of the trio>) -> ValueError about "all be given + together". + """ + with pytest.raises(ValueError, match="all be given together"): + Grid(**kwargs) + + def test_unknown_anchor_with_explicit_raises(self): + """A non-'edge' anchor on an explicit grid is rejected. + + Test scenario: + Grid(crs, resolution, bounds, anchor='center') -> ValueError. + """ + with pytest.raises(ValueError, match="anchor must be"): + Grid(crs=4326, resolution=10.0, bounds=_BOUNDS, anchor="center") + + def test_unknown_anchor_on_empty_raises(self): + """The anchor is validated even without an explicit grid. + + Test scenario: + Grid(anchor='nope') -> ValueError (anchor always checked). + """ + with pytest.raises(ValueError, match="anchor must be"): + Grid(anchor="nope") + + def test_is_frozen(self): + """Grid is immutable; attribute assignment raises. + + Test scenario: + grid.crs = 4326 -> FrozenInstanceError. + """ + grid = Grid() + with pytest.raises(dataclasses.FrozenInstanceError): + grid.crs = 4326 # type: ignore[misc] + + def test_equality(self): + """Two Grids with equal fields compare equal. + + Test scenario: + Structural equality holds for identical explicit grids. + """ + a = Grid(crs=32633, resolution=10.0, bounds=_BOUNDS) + b = Grid(crs=32633, resolution=10.0, bounds=_BOUNDS) + assert a == b, f"equal Grids should compare equal: {a} != {b}" From 75cb6f9e14753efcc3cf9e8bcd552b6f8be3f15a Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 21:41:22 +0200 Subject: [PATCH 11/12] style(dataset): apply ruff-format to _stac and test_grid --- src/pyramids/dataset/_stac.py | 4 +--- tests/dataset/test_grid.py | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pyramids/dataset/_stac.py b/src/pyramids/dataset/_stac.py index d187eacb0b..ec6939b3d7 100644 --- a/src/pyramids/dataset/_stac.py +++ b/src/pyramids/dataset/_stac.py @@ -679,9 +679,7 @@ def _point_aoi_bbox( edge_size: int, resolution: float, units: str, -) -> tuple[ - int, tuple[float, float, float, float], tuple[float, float, float, float] -]: +) -> tuple[int, tuple[float, float, float, float], tuple[float, float, float, float]]: """Compute the local-UTM EPSG, the UTM AOI, and the 4326 search bbox. The center `(lat, lon)` is reprojected to its local UTM, snapped to the diff --git a/tests/dataset/test_grid.py b/tests/dataset/test_grid.py index 96ff80f7f4..13f1a7fa92 100644 --- a/tests/dataset/test_grid.py +++ b/tests/dataset/test_grid.py @@ -28,7 +28,9 @@ def test_default_is_empty(self): assert grid.crs is None, f"crs should default None, got {grid.crs!r}" assert grid.resolution is None, f"resolution default: {grid.resolution!r}" assert grid.bounds is None, f"bounds should default None, got {grid.bounds!r}" - assert grid.anchor == "edge", f"anchor should default 'edge', got {grid.anchor!r}" + assert grid.anchor == "edge", ( + f"anchor should default 'edge', got {grid.anchor!r}" + ) assert grid.is_empty is True, "default Grid should be empty" def test_like_only_valid_and_not_empty(self): From 437b0b618f628f44c6b21c6a681799b3368c2410 Mon Sep 17 00:00:00 2001 From: Mostafa Farrag Date: Sun, 9 Aug 2026 23:33:20 +0200 Subject: [PATCH 12/12] test(collection): hoist Grid construction out of pytest.raises (S5778) Build the valid Grid before the pytest.raises block in test_oversize_grid_raises so only _resolve_target_grid's OOM guard can throw inside it -- resolves the SonarCloud python:S5778 'one throwing call per exception test' finding. --- tests/dataset/stac/test_from_stac_grid.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/dataset/stac/test_from_stac_grid.py b/tests/dataset/stac/test_from_stac_grid.py index 16b69fba3f..df649458f6 100644 --- a/tests/dataset/stac/test_from_stac_grid.py +++ b/tests/dataset/stac/test_from_stac_grid.py @@ -147,11 +147,11 @@ def test_oversize_grid_raises(self): A wide lon/lat bounds at a metre-sized resolution would be billions of pixels -> clear ValueError pointing at coarser res / like=. """ + # ~ (100 deg / 1e-4) ^2 pixels = 1e12, far over the 250M limit. The Grid + # itself is valid; only _resolve_target_grid's OOM guard should raise. + grid = Grid(crs=4326, resolution=0.0001, bounds=(0.0, 0.0, 100.0, 100.0)) with pytest.raises(ValueError, match="exceeding the"): - # ~ (100 deg / 1e-4) ^2 pixels = 1e12, far over the 250M limit. - _resolve_target_grid( - Grid(crs=4326, resolution=0.0001, bounds=(0.0, 0.0, 100.0, 100.0)) - ) + _resolve_target_grid(grid) def test_just_under_limit_builds(self): """A large-but-allowed grid still builds (S2-tile-scale).