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

Use native QGIS raster API instead of GDAL API in zonal statistics #4062

Merged
merged 2 commits into from
Feb 2, 2017
Merged

Use native QGIS raster API instead of GDAL API in zonal statistics #4062

merged 2 commits into from
Feb 2, 2017

Conversation

alexbruy
Copy link
Contributor

This should allow users to use zonal statistics with rasters which come not only from GDAL provider but also from other providers, e.g. WCS. Related issue https://hub.qgis.org/issues/8736

@Gustry
Copy link
Contributor

Gustry commented Jan 26, 2017

Thanks @alexbruy and @wonder-sk ;-)

@wonder-sk
Copy link
Member

@Gustry all credit goes to Alex, I have not done anything to implement this :-)

@Gustry
Copy link
Contributor

Gustry commented Jan 26, 2017

Ahah, I though it was with your new feature to write a raster from the QGIS api without using gdal

@nyalldawson
Copy link
Collaborator

Very nice!

Just for my own piece of mind... do you mind timing some zonal stats operations before and after this change? Just want to make sure there's no speed regression here.

@alexbruy
Copy link
Contributor Author

@nyalldawson I did quick tests using NOAAs temperature data and 2013 TIGER counties file (3234 features). Here is average result from 3 runs for each version

Using GDAL QGIS API (debug output) QGIS API (no debug output)
24590.3333 28579.3333 24591.2586

QGIS API (debug output) slower because readBlock() method in qgsgdalprovider.cpp produces some debug strings (around 5 lines for each call). If these debug strings suppressed, it shows almost the same result as GDAL version.

@nyalldawson
Copy link
Collaborator

Kill the debug code!

That's encouraging results. Another benefit I realised from this is that it means zonal stats will correctly handle rasters with an offset or scale.

+1 to merge

This should make zonal statistics usable rasters which come from
other providers, e.g. WCS.
@nyalldawson
Copy link
Collaborator

@alexbruy this all looks great to me - I say merge away!

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

4 participants