-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add overview support for imports #13
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's working; several comments only:
- minresolution and overviews created. We're setting minresolution=0 because we're computing all overviews, which is not useful in all use cases. It's better to follow the GDAL strategy, computing the minresolution overview depending on the geographical area covered. More comments below in the code.
- Stats: we should support approximate stats. In large rasters to compute exact stats is a huge overhead, and probably you can work fine with approximate stats when the purpose is visualization. The approach in this PR is computing stats block by block, which is memory safe, but it could be slow for big rasters.
- Weird issue: The colointerp capitalized is not interpreted correctly in CARTO (GDAL returns as is included here). The colortable is not used if the optional stat "top_values" doesn't exist. We can talk about that because this is something specific to CARTO.
Do you recommend that we need to do it in this PR, or is this a suggestion for future work? |
It's a suggestion to improve the script. We can add it to the backlog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment about overviews strategy following GDAL defaults
Part of #8