Major Refactor - Explicit Pixel Grids & CF Dimensions - v0.1.0 #275
Conversation
PiperOrigin-RevId: 712905652
PiperOrigin-RevId: 712996660
Ensure shape_2d is a tuple
Simplify proj params
…ion guide for v0.1.0
|
I see that there are conflicts, I'll work on resolving these. |
schwehr
left a comment
schwehr
left a comment
There was a problem hiding this comment.
The only blocker: Can we bring back python 3.10?
| matrix: | ||
| python-version: [ | ||
| "3.9", | ||
| "3.10", |
There was a problem hiding this comment.
It's great that 3.9 is out, but 3.10? I see it documented in the PR description, but we still need to support 3.10 in ee and geemap until 2026-10.
.github/workflows/publish.yml
Outdated
| - name: Set up Python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: 3.9 |
There was a problem hiding this comment.
Any reason not to use 3.13? It might make the rules run a touch faster.
Same for the other python versions used in CI rules
|
|
||
|
|
||
| assert sys.version_info >= (3, 8) | ||
| assert sys.version_info >= (3, 9) |
There was a problem hiding this comment.
assert sys.version_info >= (3, 10)| with self.assertRaises(ValueError): | ||
| ext._check_request_limit(chunks, dtype_size, xee.REQUEST_BYTE_LIMIT) | ||
|
|
||
| @mock.patch( |
There was a problem hiding this comment.
Prefer mock.patch.object
@mock.patch.object(
xee.ext.EarthEngineStore, 'get_info',
new_callable=mock.PropertyMock,
)
xee/ext_test.py
Outdated
| ) | ||
| def test_init_with_tuple_transform(self, mock_get_info): | ||
| """Test that a tuple object can be passed for crs_transform.""" | ||
| # (Setup the mock_get_info.return_value just like in the other test) |
| from pyproj import Transformer | ||
| import shapely | ||
| from shapely.ops import transform | ||
| from typing import TypedDict, Union |
There was a problem hiding this comment.
Should go with the other stdlib import (math)
xee/helpers.py
Outdated
|
|
||
| def fit_geometry( | ||
| geometry: shapely.geometry.base.BaseGeometry, | ||
| *, |
There was a problem hiding this comment.
A lot of folks do not know that this starts the required keyword section, so a comment might be good.
| | Parameter | Meaning | | ||
| |-----------|---------| | ||
| | `crs` | Coordinate Reference System for the output grid (e.g. `EPSG:4326`, `EPSG:32610`). | | ||
| | `crs_transform` | Affine transform tuple `(x_scale, x_skew, x_trans, y_skew, y_scale, y_trans)` describing pixel size, rotation/skew, and origin translation in CRS units. | |
There was a problem hiding this comment.
I think GDAL does something different: trans, scale, skew instead of scale, skew, trans.
Should we mention this is the rasterio/affine convention?
|
|
||
| ## Dimension Ordering | ||
|
|
||
| Datasets are returned as `[time, y, x]` (v1.0+) aligning with CF conventions and most geospatial libraries. Prior versions used `[time, x, y]`. If code assumed positional indices, update to name-based access: `ds.sizes['x']`, `ds.sizes['y']`. |
There was a problem hiding this comment.
What is this v1.0+ here?
|
|
||
| ## CRS Units & Transforms | ||
|
|
||
| All scale/translation values are expressed in units of `crs`. Degrees for geographic CRSs; meters (or feet) for projected CRSs. Plate Carrée (`EPSG:4326`) has non-uniform ground size — consider a projected CRS for area/length sensitive analysis. |
There was a problem hiding this comment.
Is "Plate Carrée" standard technical language for WGS84? Given my experience, I can't tell whether this is a useful name to include here or "AI slop".
| grid_params = helpers.fit_geometry( | ||
| geometry=sf_aoi_shapely, | ||
| grid_crs='EPSG:32610', # Target CRS in meters (UTM Zone 10N) | ||
| grid_scale=(30, -30) # Use Landsat's 30m resolution |
There was a problem hiding this comment.
Minor nit: Extra horizontal space here.
| - [Core Concepts](concepts.md) | ||
| - [Performance & Limits](performance.md) | ||
| - [FAQ](faq.md) | ||
| - Examples: see `examples/` directory in the repository |
There was a problem hiding this comment.
Like the other points, consider linking to the examples/ directory.
|
|
||
| #### Example 1: Global dataset at fixed scale | ||
|
|
||
| **Before (v0.x):** |
There was a problem hiding this comment.
Technically v0.1.0 fits this expression. It's probably better to use v0.0.x here and elsewhere.
| # data as a single chunk. | ||
| Chunks = Union[int, dict[Any, Any], Literal['auto'], None] | ||
|
|
||
| # Types for type hints |
There was a problem hiding this comment.
"type annotations". I don't think this comment is necessary though.
xee/ext.py
Outdated
| 'shearX', | ||
| 'translateX', | ||
| 'shearY', | ||
| 'scaleY', |
There was a problem hiding this comment.
scale, shear, translate? X and Y transform orders are different.
I'm a little surprised the integration tests didn't catch this.
| [123, 0, 100, 0, 456, 200] | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
Minor nit: extra vertical space after some of these test cases.
4 participants
Major Refactor - Explicit Pixel Grids & CF Dimensions
This PR represents a major refactor to the core of Xee's backend. It is merges the
simplify_pixel_grid_paramsbranch into main. Thesimplify_pixel_grid_paramshas months of changes - the significant changes to Xee API have been through review, doc changes and CI/CD Python version changes were directly pushed. The most significant changes are adopting explicit pixel grid parameters for opening datasets (see discussion) and updating the default dimension ordering to align with community standards (see discussion). After this PR is merged we should release v0.1.0 (from v0.0.x).xarray.open_datasetcalls when upgrading to this version. Please refer to the Migration Guide for detailed instructions.💥 Breaking Changes & Rationale
1. Explicit Pixel Grid Definition
The previous, heuristic-based grid definition arguments (
scale,geometry, andprojection) have been removed fromxr.open_dataset(..., engine='ee').open_datasetfunction now requires three explicit parameters to define the pixel grid:crs,crs_transform(a 6-tuple affine matrix), andshape_2d(width/height pixel count).2. CF-friendly Dimension Ordering
The default order of spatial dimensions in the resulting Xarray objects has been changed.
[time, y, x]instead of the old[time, x, y]..transpose()calls.✨ New Features & Helper Utilities
To support the new explicit grid workflow, a new
xee.helpersmodule has been added with key utilities:extract_grid_params(ee_obj): Automatically derives the requiredcrs,crs_transform, andshape_2dfrom an existingee.Imageoree.ImageCollection.fit_geometry(...): Computes the required grid parameters to cover a specificshapely.geometry(AOI) at either a fixed scale/resolution or a fixed shape/pixel count.math.floorandmath.ceilto precisely snap the grid to the bounding box extents, improving coordinate accuracy and preventing sub-pixel misalignment.📚 Documentation & Infrastructure Updates
docs/migration-guide-v0.1.0.md, has been created to assist users in updating their code to the new v0.1.0 API.concepts.md) and a User Guide (guide.md) were added to clarify the philosophy behind the pixel grid parameters and collect common workflows. The mainREADME.mdand examples have also been fully updated.3.10support was removed from all CI workflows, and the default publish environment was updated to Python3.11.📋 Checklist
PixelGridParamssignature and removed old implicit arguments.[time, y, x].extract_grid_params,fit_geometry, andset_scale.ext_integration_test.pyandext_integration_test.pypass.