feat: update ee_to_xarray() to support xee v0.1.0 API with backward compatibility#2782
feat: update ee_to_xarray() to support xee v0.1.0 API with backward compatibility#2782jdbcode wants to merge 3 commits into
Conversation
…ompatibility - Convert legacy scale/geometry parameters to new grid_params format - Use xee.helpers.fit_geometry() and extract_grid_params() for grid system - Handle ee.Geometry to shapely.geometry conversion - Detect geographic vs projected CRS for proper scale handling - Maintain backward compatibility with xee v0.0.x - Add helpful warnings if conversion fails - Closes #2780
- Add test for legacy scale parameter conversion - Add test for geometry parameter handling - Both tests validate proper grid parameter generation and old parameter removal
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Code Review
This pull request updates the ee_to_xarray function to support the xee v0.1.0+ API by automatically converting legacy parameters into the new grid parameter system and adds corresponding unit tests. Review feedback suggests several improvements, including handling ee.Projection objects to prevent potential attribute errors, simplifying redundant logic in conditional checks and scale assignments, and using shapely.geometry.shape() for more robust geometry parsing.
| ) | ||
|
|
||
| # Determine if we need to convert legacy parameters to grid_params. | ||
| # Also handle no-grid input by defaulting to source/native grid. |
There was a problem hiding this comment.
The logic in this if statement contains a redundant check for scale is not None. Since scale is not None is the first part of the or condition, the second check inside the parenthesis will only be evaluated if scale is already None.
| # Also handle no-grid input by defaulting to source/native grid. | |
| if scale is not None or (crs is not None and geometry is not None): |
| raise ImportError( | ||
| "shapely is required to convert legacy grid parameters " | ||
| "for xee>=0.1.0" | ||
| ) |
There was a problem hiding this comment.
The crs parameter can be an ee.Projection object. If it is, target_crs will be an object that does not support the .upper() method used later on line 3153, leading to an AttributeError. It is safer to extract the CRS string if an ee.Projection is provided.
if isinstance(crs, ee.Projection):
target_crs = crs.getInfo().get("crs", "EPSG:4326")
else:
target_crs = crs or "EPSG:4326"| # Determine the geometry to use | ||
| if geometry is not None: | ||
| # Convert EE geometry to shapely if needed | ||
| if isinstance(geometry, ee.Geometry): | ||
| geom_dict = geometry.getInfo() | ||
| if geom_dict["type"] == "Polygon": | ||
| coords = geom_dict["coordinates"][0] | ||
| geom_shapely = shapely.geometry.Polygon(coords) | ||
| elif geom_dict["type"] == "Rectangle": | ||
| geom_shapely = shapely.geometry.box(*geom_dict["coordinates"]) | ||
| else: | ||
| raise ValueError( |
There was a problem hiding this comment.
Instead of manually parsing specific geometry types, you can use shapely.geometry.shape() which is more robust and handles all GeoJSON-compatible geometry types (including holes in Polygons, MultiPolygons, etc.) automatically.
if isinstance(geometry, ee.Geometry):
geom_dict = geometry.getInfo()
geom_shapely = shapely.geometry.shape(geom_dict)| geom_shapely = shapely.geometry.box(-180, -90, 180, 90) | ||
|
|
||
| # Convert scale to (x_scale, y_scale) tuple | ||
| if scale is not None: | ||
| # Determine if scale is in degrees (geographic) or meters (projected) | ||
| if "EPSG:4326" in target_crs or "WGS" in target_crs.upper(): | ||
| # Geographic CRS - scale is in degrees | ||
| grid_scale = (scale, -scale) |
There was a problem hiding this comment.
The if/else block inside if scale is not None is redundant because both branches perform the exact same assignment: grid_scale = (scale, -scale). The CRS check is only necessary when determining the default scale.
| geom_shapely = shapely.geometry.box(-180, -90, 180, 90) | |
| # Convert scale to (x_scale, y_scale) tuple | |
| if scale is not None: | |
| # Determine if scale is in degrees (geographic) or meters (projected) | |
| if "EPSG:4326" in target_crs or "WGS" in target_crs.upper(): | |
| # Geographic CRS - scale is in degrees | |
| grid_scale = (scale, -scale) | |
| if scale is not None: | |
| grid_scale = (scale, -scale) |
There was a problem hiding this comment.
Pull request overview
Updates geemap.ee_to_xarray() to support xee v0.1.0’s new grid-parameter API (crs_transform, shape_2d) while attempting to preserve legacy call patterns (crs/scale/geometry) via conversion and auto-inference.
Changes:
- Added a compatibility layer in
ee_to_xarray()to (1) convert legacy grid args to v0.1.0 grid params and (2) infer native grid params when none are provided. - Made
shapelyan optional import used for legacy-parameter conversion. - Added unit tests intended to validate legacy
scaleandgeometryconversions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| geemap/common.py | Adds xee v0.1.0 grid-param compatibility logic (conversion + inference) to ee_to_xarray(). |
| tests/test_common.py | Adds tests for legacy scale and geometry call patterns for ee_to_xarray(). |
Comments suppressed due to low confidence (4)
geemap/common.py:3111
- The legacy-conversion condition doesn’t trigger when callers pass only
geometry(with neitherscalenorcrs). In that case, the code falls through toextract_grid_params()inference and silently ignores the providedgeometry, which is a backward-compat behavior change from the legacy API wheregeometryconstrained bounds. Consider treatinggeometryalone as legacy usage (e.g., infer native grid first, thenfit_geometryto the provided geometry) or raising a clear error telling users to passscale/explicit grid params when usinggeometry.
# Determine if we need to convert legacy parameters to grid_params.
# Also handle no-grid input by defaulting to source/native grid.
if scale is not None or (
geemap/common.py:3136
- The EE-geometry conversion only supports
PolygonandRectangleand drops inner rings/holes (it uses onlycoordinates[0]). Previously,geometrywas forwarded to xee and could support other EE geometry types, so this narrows backward compatibility and can produce incorrect bounds for polygons with holes. Use a generic GeoJSON→Shapely conversion (e.g.,shapely.geometry.shape(geom_dict)) so all geometry types and rings are handled correctly.
# Determine the geometry to use
if geometry is not None:
# Convert EE geometry to shapely if needed
if isinstance(geometry, ee.Geometry):
geom_dict = geometry.getInfo()
if geom_dict["type"] == "Polygon":
coords = geom_dict["coordinates"][0]
geom_shapely = shapely.geometry.Polygon(coords)
elif geom_dict["type"] == "Rectangle":
geom_shapely = shapely.geometry.box(*geom_dict["coordinates"])
else:
raise ValueError(
tests/test_common.py:781
mock_geometry = mock.MagicMock(spec=ee.Geometry)is not an actualee.Geometryinstance, soisinstance(geometry, ee.Geometry)will be false inee_to_xarray(), causing aTypeErrorin the conversion path. Use a real geometry object that passes the type check (e.g., construct anee.Geometry.Polygon(...)if feasible in tests, or use the existingtests.fake_ee.Geometry()pattern and adjust the production check accordingly), or change the production code to accept geometry-like objects via duck-typing (hasattr(getInfo)) if that’s intended.
mock_ds = xr.Dataset()
mock_open_ds.return_value = mock_ds
# Mock ee.Geometry
mock_geometry = mock.MagicMock(spec=ee.Geometry)
geemap/common.py:3180
- When legacy→new conversion fails (including when
shapelyis missing), the code only emits a warning and then falls back to passing legacy params (scale/geometry/projection) through to xee. With xee>=0.1.0 this is guaranteed to raise (e.g., “unexpected keyword argument 'scale'”), so the warning+fallback path just defers a harder-to-understand failure. Consider raising a clear exception immediately when conversion fails and legacy params are present (especially forImportErrorfrom missing shapely), with guidance to install shapely or pass explicitcrs_transform/shape_2d.
grid_crs=target_crs,
grid_scale=grid_scale,
)
except Exception as e:
import warnings
warnings.warn(
f"Failed to convert legacy API parameters to new grid format: {e}. "
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| has_explicit_grid_params = all( | ||
| k in kwargs and kwargs[k] is not None | ||
| for k in ["crs", "crs_transform", "shape_2d"] |
| from xee import helpers | ||
|
|
||
| try: | ||
| import shapely |
| projection (optional): An `ee.Projection` object (legacy API, xee v0.0.x). | ||
| Automatically converted to `crs` and `scale` for the new API. | ||
| geometry (optional): An `ee.Geometry` or shapely geometry to define the | ||
| region of interest. Converted to the new grid system for xee v0.1.0+. |
| @mock.patch("geemap.common.xr.open_dataset") | ||
| @mock.patch("geemap.common.ee.Initialize") | ||
| @mock.patch("geemap.common.ee.data.is_initialized", return_value=True) | ||
| def test_ee_to_xarray_with_scale_parameter( | ||
| self, mock_is_init, mock_init, mock_open_ds | ||
| ): | ||
| """Test ee_to_xarray with legacy scale parameter (xee v0.0.x API).""" | ||
| # Mock xr.open_dataset to return a dummy dataset | ||
| import xarray as xr |
|
🚀 Deployed on https://6a051e819fd7b98bba57c2e1--opengeos.netlify.app |
Closes #2780
Summary
Updates
geemap.ee_to_xarray()to work with the breaking API changes introduced in xee v0.1.0, while remaining fully backward compatible with existing user code.What changed in xee v0.1.0
xee v0.1.0 replaced the old grid parameters (
scale,geometry) with explicit grid parameters (crs_transform,shape_2d). Calls using the old API raise errors with the new xee version.Changes in this PR
geemap/common.py—ee_to_xarray():The function now implements a three-path compatibility layer:
crs_transformandshape_2dare already inkwargs, pass them directly to xee (new API users).scaleand/orgeometryare provided, convert them to the newcrs_transform/shape_2dgrid params usingxee.helpers.fit_geometry(). This preserves full backward compatibility.ee_to_xarray("projects/...")with just an asset ID), infer the native source grid viaxee.helpers.extract_grid_params(). This is the most common usage pattern.Additional changes:
shapelyis now an optional import (graceful fallback with a clear error message if missing and legacy params are used)crsparameter is correctly preserved when converting legacy params (was accidentally removed in the original path)tests/test_common.py:test_ee_to_xarray_with_scale_parameter— verifiesscale→ new grid params conversiontest_ee_to_xarray_with_geometry— verifiesscale+geometry→ new grid params conversionBackward compatibility
All existing call patterns continue to work:
ee_to_xarray(asset_id)ee_to_xarray(asset_id, crs=..., scale=...)ee_to_xarray(asset_id, crs=..., scale=..., geometry=...)ee_to_xarray(image_collection)ee_to_xarray(image)ee_to_xarray(..., crs_transform=..., shape_2d=...)Known limitation: Passing a
listof asset IDs still does not work with xee (this is a pre-existing limitation unrelated to the v0.1.0 migration).