From 7b747cbdf86bcc13766074aba2573a57e6511606 Mon Sep 17 00:00:00 2001 From: Kirill Kouzoubov Date: Tue, 8 Mar 2022 13:26:39 +1100 Subject: [PATCH 1/2] Backport CRS fixes from odc-geo (#1230) This is mostly to address slow hashing issue #1230. But also tackles slow construction described in #1222 without moving away from epsg representation as a default when available. --- datacube/utils/geometry/_base.py | 79 ++++++++++++++++++-------------- tests/test_geometry.py | 6 +-- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/datacube/utils/geometry/_base.py b/datacube/utils/geometry/_base.py index 434541591a..9b1d559cef 100644 --- a/datacube/utils/geometry/_base.py +++ b/datacube/utils/geometry/_base.py @@ -116,10 +116,25 @@ def from_points(p1: Tuple[float, float], (p1[1], p2[1])) -@cachetools.cached({}) -def _make_crs(crs_str: str) -> Tuple[_CRS, Optional[int]]: - crs = _CRS.from_user_input(crs_str) - return (crs, crs.to_epsg()) +def _make_crs_key(crs_spec: Union[str, _CRS]) -> str: + if isinstance(crs_spec, str): + normed_epsg = crs_spec.upper() + if normed_epsg.startswith("EPSG:"): + return normed_epsg + return crs_spec + return crs_spec.to_wkt() + + +@cachetools.cached({}, key=_make_crs_key) +def _make_crs(crs: Union[str, _CRS]) -> Tuple[_CRS, str, Optional[int]]: + if isinstance(crs, str): + crs = _CRS.from_user_input(crs) + epsg = crs.to_epsg() + if epsg is not None: + crs_str = f"EPSG:{epsg}" + else: + crs_str = crs.to_wkt() + return (crs, crs_str, crs.to_epsg()) def _make_crs_transform_key(from_crs, to_crs, always_xy): @@ -131,26 +146,6 @@ def _make_crs_transform(from_crs, to_crs, always_xy): return Transformer.from_crs(from_crs, to_crs, always_xy=always_xy).transform -def _guess_crs_str(crs_spec: Any) -> Optional[str]: - """ - Returns a string representation of the crs spec. - Returns `None` if it does not understand the spec. - """ - if isinstance(crs_spec, str): - return crs_spec - if isinstance(crs_spec, dict): - crs_spec = _CRS.from_dict(crs_spec) - - if hasattr(crs_spec, 'to_wkt'): - return crs_spec.to_wkt() - if hasattr(crs_spec, 'to_epsg'): - epsg = crs_spec.to_epsg() - if epsg is not None: - return 'EPSG:{}'.format(crs_spec.to_epsg()) - - return None - - class CRS: """ Wrapper around `pyproj.CRS` for backwards compatibility. @@ -160,20 +155,34 @@ class CRS: __slots__ = ('_crs', '_epsg', '_str') - def __init__(self, crs_str: Any): + def __init__(self, crs_spec: Any): """ :param crs_str: string representation of a CRS, often an EPSG code like 'EPSG:4326' :raises: `pyproj.exceptions.CRSError` """ - crs_str = _guess_crs_str(crs_str) - if crs_str is None: - raise CRSError("Expect string or any object with `.to_epsg()` or `.to_wkt()` method") - - _crs, _epsg = _make_crs(crs_str) - - self._crs = _crs - self._epsg = _epsg - self._str = crs_str + if isinstance(crs_spec, str): + self._crs, self._str, self._epsg = _make_crs(crs_spec) + elif isinstance(crs_spec, CRS): + self._crs = crs_spec._crs + self._epsg = crs_spec._epsg + self._str = crs_spec._str + elif isinstance(crs_spec, _CRS): + self._crs, self._str, self._epsg = _make_crs(crs_spec) + elif isinstance(crs_spec, dict): + self._crs, self._str, self._epsg = _make_crs(_CRS.from_dict(crs_spec)) + else: + _to_epsg = getattr(crs_spec, "to_epsg", None) + if _to_epsg is not None: + self._crs, self._str, self._epsg = _make_crs(f"EPSG:{_to_epsg()}") + return + _to_wkt = getattr(crs_spec, "to_wkt", None) + if _to_wkt is not None: + self._crs, self._str, self._epsg = _make_crs(_to_wkt()) + return + + raise CRSError( + "Expect string or any object with `.to_epsg()` or `.to_wkt()` methods" + ) def __getstate__(self): return {'crs_str': self._str} @@ -257,7 +266,7 @@ def __str__(self) -> str: return self._str def __hash__(self) -> int: - return hash(self.to_wkt()) + return hash(self._str) def __repr__(self) -> str: return "CRS('%s')" % self._str diff --git a/tests/test_geometry.py b/tests/test_geometry.py index 1c989e2023..235ba46430 100644 --- a/tests/test_geometry.py +++ b/tests/test_geometry.py @@ -43,12 +43,12 @@ bounding_box_in_pixel_domain, geobox_intersection_conservative, geobox_union_conservative, - _guess_crs_str, force_2d, _align_pix, _round_to_res, _norm_crs, _norm_crs_or_error, + _make_crs_key, ) from datacube.testutils.geom import ( epsg4326, @@ -1475,9 +1475,9 @@ def test_crs_hash(): def test_base_internals(): - assert _guess_crs_str(CRS("epsg:3577")) == epsg3577.to_wkt() + assert _make_crs_key("epsg:3577") == "EPSG:3577" no_epsg_crs = CRS(SAMPLE_WKT_WITHOUT_AUTHORITY) - assert _guess_crs_str(no_epsg_crs) == no_epsg_crs.to_wkt() + assert _make_crs_key(no_epsg_crs.proj) == no_epsg_crs.proj.to_wkt() gjson_bad = {'type': 'a', 'coordinates': [1, [2, 3, 4]]} assert force_2d(gjson_bad) == {'type': 'a', 'coordinates': [1, [2, 3]]} From 426e411d80a9f06a116560344a43831b93469ca2 Mon Sep 17 00:00:00 2001 From: Kirill Kouzoubov Date: Tue, 8 Mar 2022 13:33:02 +1100 Subject: [PATCH 2/2] Remove overly strict assert in GeoBox (#1235) Non-axis aligned geoboxes are supported actually, only coordinate computation assumes axis-aligned geobox. So move assert into there. --- datacube/utils/geometry/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datacube/utils/geometry/_base.py b/datacube/utils/geometry/_base.py index 9b1d559cef..41b906efce 100644 --- a/datacube/utils/geometry/_base.py +++ b/datacube/utils/geometry/_base.py @@ -996,7 +996,6 @@ class GeoBox: """ def __init__(self, width: int, height: int, affine: Affine, crs: MaybeCRS): - assert is_affine_st(affine), "Only axis-aligned geoboxes are currently supported" self.width = width self.height = height self.affine = affine @@ -1119,6 +1118,7 @@ def coordinates(self) -> Dict[str, Coordinate]: """ dict of coordinate labels """ + assert is_affine_st(self.affine), "Only axis-aligned geoboxes are currently supported" yres, xres = self.resolution yoff, xoff = self.affine.yoff, self.affine.xoff