refactor(collection)!: consolidate the DatasetCollection constructor API - #951
Merged
Conversation
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.
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=...).
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.
MAfarrag
changed the title
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.
…fy 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).
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.
… 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=(...)).
…grid Extend the Grid mechanism to from_point: it now builds a Grid(crs=<local UTM>, resolution, bounds=<resolution-snapped UTM AOI>) 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.
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.
…/dataset-collection
MAfarrag
changed the title
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.
|
MAfarrag
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Consolidates the
DatasetCollectionconstructor surface, adds aGridvalue object, and completes thefrom_pointgrid match. Plus docs and tests.
DatasetCollection.create_cube->from_dataset(paramsrc->dataset) to align with thefrom_*readerfamily and disambiguate it from
Dataset.create; hard rename, no deprecation alias.Grid, a frozen dataclass exported frompyramids.datasetwhose__post_init__enforces the target-grid modeinvariants (
likexor thecrs/resolution/boundstrio, the trio all-or-nothing, andanchor).from_stac's five target-grid kwargs (like,crs,resolution,bounds,anchor) into a singlegrid: Grid | Noneparameter, dropping it from 14 to 10 params and mirroringodc.stac.load'sgeobox=grouping.from_pointcubes onto the exactedge_sizexedge_sizelocal-UTM grid via an internally builtGrid,closing the documented native-grid gap.
Gridunit suite; migrate the grid-match tests.Issues
create_cubetofrom_datasetfrom_stactarget-grid params into aGriddataclassfrom_pointcubes onto the exact local-UTM gridRelates to #757 (same python:S107 parameter-count reduction, applied to
from_stac).Type of change
Check relevant points.
How Has This Been Tested?
Targeted suites run against the branch (
MPLBACKEND=Agg):tests/dataset/test_grid.py— dedicatedGridunit suite (construction, invariants, is_empty, frozen, equality)tests/dataset/stac/test_from_stac_grid.py— resolver +from_stacgrid match viaGridtests/dataset/collection/test_from_point.py— asserts the forwardedGrid(local UTM crs/resolution/bounds)Griddoctests pass;mypyclean on the changed source; pre-commit (ruff/bandit/format) green; SonarCloud cleanChecklist:
BREAKING CHANGE:
DatasetCollection.create_cubeis renamed tofrom_dataset, andfrom_stacno longer acceptslike/crs/resolution/bounds/anchor— passgrid=Grid(...)instead.