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

Add minimal Python bindings #53

Merged
merged 20 commits into from
Dec 7, 2023
Merged

Conversation

dbaston
Copy link
Member

@dbaston dbaston commented Dec 1, 2023

I'm excited to have had some availability to work on the Python bindings. Talking to some users at FOSS4G-NA, I would say the enthusiasm for moving exactextract directly into the GDAL Python bindings was not universal -- there was more interest in using exactextract with other libraries like rasterio and xarray. So I decided to stick to the original plan and use pybind11. I began with the bindings put together by @jdalrym2 as a starting point and made the following changes:

  • adapted to fit changes in internal interfaces (e.g., use of Feature)
  • removed bindings for internal classes (Cell, Coordinate, etc.)
  • removed bindings for GDAL-dependent classes (GDALRasterWrapper, etc.) so that we do not have GDAL as a build or runtime requirement
  • added adapter classes to define FeatureSource and RasterSource objects in Python. As examples, the GDALFeatureSource and GDALRasterSource classes use the GDAL Python bindings to read vector and raster data. You can also stick an arbitrary NumPy array into a NumPyRasterSource and a list of GeoJSON features into a JSONFeatureSource if you're not using GDAL. It should be straightforward to write adapters for inputs associated with other common libraries.
  • first steps at adding a high-level interface mimicking the one available in R (https://isciences.gitlab.io/exactextractr/reference/exact_extract.html)

Some components of @jdalrym2's effort are completely missing here, such as documentation and a setup.py configuration. There are clearly needed but I wanted to keep the scope of this PR manageable and, for documentation, I think it makes sense to let the interfaces settle down a bit first. In particular, I plan to implement support for efficiently processing multi-band rasters, like we have in exactextractr. It's possible that this will force some C++ API changes that impact Python bindings.

Because the commit history got messy, I've squashed the bindings work down to a single commit with me and @jdalrym2 as co-authors.

@dbaston dbaston mentioned this pull request Dec 1, 2023
@BwL1289
Copy link

BwL1289 commented Dec 1, 2023

Thank you for all the hard work, Dan.

@jdalrym2
Copy link
Contributor

jdalrym2 commented Dec 1, 2023

Thanks for the credit @dbaston! I'm happy to see this work come into fruition. I'll find some time to review today/tomorrow in case I might have any insight for you. 🙂

@dbaston dbaston force-pushed the add-pybindings branch 2 times, most recently from 7beb4dc to 4c61a9e Compare December 5, 2023 17:40
Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Attention: 23 lines in your changes are missing coverage. Please review.

Comparison is base (f108722) 50.37% compared to head (6e62988) 50.77%.

Files Patch % Lines
src/feature.cpp 17.64% 14 Missing ⚠️
src/processor.h 37.50% 5 Missing ⚠️
src/operation.h 77.77% 2 Missing ⚠️
src/deferred_gdal_writer.cpp 83.33% 1 Missing ⚠️
src/raster_sequential_processor.cpp 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   50.37%   50.77%   +0.39%     
==========================================
  Files          72       72              
  Lines       11614    11643      +29     
==========================================
+ Hits         5851     5912      +61     
+ Misses       5763     5731      -32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbaston dbaston merged commit 6f0c759 into isciences:master Dec 7, 2023
10 checks passed
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.

3 participants