PERF: CRS - use WKT for cache and lazy load EPSG code #1322

snowman2 · 2022-10-10T21:21:41Z

If this is merged, I will be happy to port it into odc-geo.

snowman2 · 2022-10-10T21:22:10Z

datacube/utils/geometry/_base.py

    return crs_spec.to_wkt()


 @cachetools.cached({}, key=_make_crs_key)  # type: ignore[misc]
-def _make_crs(crs: Union[str, _CRS]) -> Tuple[_CRS, str, Optional[int]]:
-    if isinstance(crs, str):
+def _make_crs(crs: Union[str, int, _CRS]) -> Tuple[_CRS, str, Optional[int]]:


Allow passing in integer EPSG code.

snowman2 · 2022-10-10T21:22:25Z

datacube/utils/geometry/_base.py

        crs = _CRS.from_user_input(crs)
-    epsg = crs.to_epsg()


Remove all to_epsg calls.

snowman2 · 2022-10-10T21:23:08Z

datacube/utils/geometry/_base.py

            try:
                wkt = crs_spec.to_wkt()
            except AttributeError:
                wkt = None
            if wkt is not None:
                self._crs, self._str, self._epsg = _make_crs(wkt)
                return
+            try:
+                epsg = crs_spec.to_epsg()


Use EPSG as backup to WKT. Likely never used. Will raise exception in the future if no WKT (see: pyproj4/pyproj#1036).

datacube/utils/geometry/_base.py

snowman2 · 2022-10-10T21:30:28Z

Note: I will be happy to update the tests if this is the desired pattern to move forward with.

SpacemanPaul · 2022-10-11T03:23:26Z

Note: I will be happy to update the tests if this is the desired pattern to move forward with.

I will have a look at the tests later this week - I suspect some of them are not your fault.

SpacemanPaul · 2022-10-13T02:07:10Z

@snowman2 I think a good chunk of the test issues are due to this no longer working:

from datacube.utils.geometry import CRS
crs = CRS("EPSG:4326")

assert crs.epsg == 4326

With this PR crs.epsg returns None.

SpacemanPaul

I think the changes I suggest should just about do it. We might need to make some changes to the (experimental) postgis tests as well.

datacube/utils/geometry/_base.py

snowman2 · 2022-10-13T14:41:22Z

Good catches @SpacemanPaul 👍. I updated the PR based on your comments.

codecov · 2022-10-13T15:12:50Z

Codecov Report

Base: 92.34% // Head: 92.33% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (d2d7d0d) compared to base (e0dfd94).
Patch coverage: 86.36% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1322      +/-   ##
===========================================
- Coverage    92.34%   92.33%   -0.01%     
===========================================
  Files          127      127              
  Lines        13581    13585       +4     
===========================================
+ Hits         12541    12544       +3     
- Misses        1040     1041       +1

Impacted Files	Coverage Δ
datacube/utils/geometry/_base.py	`99.34% <85.71%> (-0.26%)`	⬇️
datacube/drivers/postgis/_spatial.py	`94.31% <100.00%> (+1.13%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

snowman2 · 2022-10-13T15:12:59Z

datacube/utils/geometry/_base.py

+            epsg = int(crs_str.split(":", maxsplit=1)[-1])
+        else:
+            crs_str = crs
+    crs = _CRS.from_user_input(crs)


Always call _CRS.from_user_input as it supports all of these types. Also, if it is a _CRS, it just returns the original _CRS ref.

snowman2 · 2022-10-13T16:07:40Z

datacube/utils/geometry/_base.py

+def _make_crs(crs: Union[str, int, _CRS]) -> Tuple[_CRS, str, Optional[int]]:
+    epsg = False
+    crs = _CRS.from_user_input(crs)
+    crs_str = crs.srs


srs is the attribute for the user input string.

SpacemanPaul

Thanks heaps @snowman2

snowman2 · 2022-10-13T23:50:59Z

Thanks @SpacemanPaul 👍

PERF: CRS - use WKT for cache and lazy load EPSG code

3ee4164

snowman2 commented Oct 10, 2022

View reviewed changes

datacube/utils/geometry/_base.py

crs = _CRS.from_user_input(crs)

epsg = crs.to_epsg()

Copy link

Contributor Author

snowman2 Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all to_epsg calls.

snowman2 commented Oct 10, 2022

View reviewed changes

datacube/utils/geometry/_base.py Show resolved Hide resolved

SpacemanPaul requested a review from Kirill888 October 10, 2022 23:01

SpacemanPaul self-requested a review October 13, 2022 07:11

SpacemanPaul requested changes Oct 13, 2022

View reviewed changes

datacube/utils/geometry/_base.py Outdated Show resolved Hide resolved

datacube/utils/geometry/_base.py Show resolved Hide resolved

snowman2 force-pushed the crs_wkt_cache branch 3 times, most recently from ade84ea to 0e6f0aa Compare October 13, 2022 15:02

snowman2 commented Oct 13, 2022

View reviewed changes

snowman2 force-pushed the crs_wkt_cache branch from 0e6f0aa to 1b5cf8b Compare October 13, 2022 16:01

snowman2 commented Oct 13, 2022

View reviewed changes

Alan Snow added 2 commits October 13, 2022 11:43

PR comment fixes

a7bb209

adjust spindex_for_crs for WKT based CRS loading

d2d7d0d

snowman2 force-pushed the crs_wkt_cache branch from 1b5cf8b to d2d7d0d Compare October 13, 2022 16:43

SpacemanPaul approved these changes Oct 13, 2022

View reviewed changes

SpacemanPaul merged commit 0535335 into opendatacube:develop Oct 13, 2022

snowman2 deleted the crs_wkt_cache branch October 13, 2022 23:50

snowman2 mentioned this pull request Oct 14, 2022

PERF: CRS - use WKT for cache and lazy load EPSG code opendatacube/odc-geo#70

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PERF: CRS - use WKT for cache and lazy load EPSG code #1322

PERF: CRS - use WKT for cache and lazy load EPSG code #1322

snowman2 commented Oct 10, 2022 •

edited

snowman2 Oct 10, 2022

snowman2 Oct 10, 2022

snowman2 Oct 10, 2022 •

edited

snowman2 commented Oct 10, 2022

SpacemanPaul commented Oct 11, 2022

SpacemanPaul commented Oct 13, 2022 •

edited

SpacemanPaul left a comment

snowman2 commented Oct 13, 2022

codecov bot commented Oct 13, 2022 •

edited

snowman2 Oct 13, 2022

snowman2 Oct 13, 2022

SpacemanPaul left a comment

snowman2 commented Oct 13, 2022

PERF: CRS - use WKT for cache and lazy load EPSG code #1322

PERF: CRS - use WKT for cache and lazy load EPSG code #1322

Conversation

snowman2 commented Oct 10, 2022 • edited

snowman2 Oct 10, 2022

Choose a reason for hiding this comment

snowman2 Oct 10, 2022

Choose a reason for hiding this comment

snowman2 Oct 10, 2022 • edited

Choose a reason for hiding this comment

snowman2 commented Oct 10, 2022

SpacemanPaul commented Oct 11, 2022

SpacemanPaul commented Oct 13, 2022 • edited

SpacemanPaul left a comment

Choose a reason for hiding this comment

snowman2 commented Oct 13, 2022

codecov bot commented Oct 13, 2022 • edited

Codecov Report

snowman2 Oct 13, 2022

Choose a reason for hiding this comment

snowman2 Oct 13, 2022

Choose a reason for hiding this comment

SpacemanPaul left a comment

Choose a reason for hiding this comment

snowman2 commented Oct 13, 2022

snowman2 commented Oct 10, 2022 •

edited

snowman2 Oct 10, 2022 •

edited

SpacemanPaul commented Oct 13, 2022 •

edited

codecov bot commented Oct 13, 2022 •

edited