Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

snowman2
Copy link
Contributor

@codecov
Copy link

codecov bot commented Oct 14, 2022

Codecov Report

Base: 98.19% // Head: 98.14% // Decreases project coverage by -0.05% ⚠️

Coverage data is based on head (d5779b5) compared to base (9282c21).
Patch coverage: 89.47% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #70      +/-   ##
===========================================
- Coverage    98.19%   98.14%   -0.06%     
===========================================
  Files           23       23              
  Lines         3768     3768              
===========================================
- Hits          3700     3698       -2     
- Misses          68       70       +2     
Impacted Files Coverage Δ
odc/geo/crs.py 99.16% <89.47%> (-0.84%) ⬇️

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.

@Kirill888
Copy link
Member

@snowman2 thanks for this, but I'm not too sure this is the right way to approach this problem. I think rasterio.crs.CRS is a common enough input type and we should handle it in the _make_crs cached path, rather than handling it in a "generic" section. rasterio.crs.CRS is a hashable object so can be used as a key directly, only issue is that I want odc-geo to be independent from rasterio, so it might be a bit trickier to implement because of that.

I'm guessing that a performant way to handle rasterio.crs.CRS object would be to query .is_epsg_code property before attempting to call .to_epsg, also with those things going through cache performance impact will be reduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants