thinkingmachines / geowrangler

🌏 A python package for wrangling geospatial datasets
https://geowrangler.thinkingmachin.es/
MIT License
47 stars 14 forks source link

Exactextract zonal stats implementation #236

Closed tm-jc-nacpil closed 2 months ago

tm-jc-nacpil commented 2 months ago

Intro

This PR adds an initial implementation of exactextract zonal stats

  1. Adds create_exactextract_zonal_stats() function
  2. Adds data/ph_s5p_AER_AI_340_380.tiff which is a multiband raster for demos. (Sentinel 5P absorbing aerosol index over PH)

Implementation Notes

The current exactextract function is able to do multiband calculations using a similar format to vector_zonal_stats. Would like to note some things here

  1. The data to be aggregated should be specified as band parameter.
    • If the passed band number is outside the valid range based on the band count, I elected to skip it for now and print a warning. This is different from vector zonal stats which applies default data columns (index), but I was thinking this is safer so that the user is aware that the passed band is wrong
  2. The output column specifies the output column names

Current to-dos

review-notebook-app[bot] commented 2 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

tm-jc-nacpil commented 2 months ago

Re: NODATA value Just realized out that the implementation for nodata / default value handling is not yet in the latest pypi release 😭 See: https://github.com/isciences/exactextract/issues/91 (dated May) but the latest release was in March

For this, I was thinking we can just document that it isn't supported yet, then we can revisit once exactextract in pypi is updated again? 🤔

tm-danna-ang commented 2 months ago

Re: NODATA value Just realized out that the implementation for nodata / default value handling is not yet in the latest pypi release 😭 See: isciences/exactextract#91 (dated May) but the latest release was in March

For this, I was thinking we can just document that it isn't supported yet, then we can revisit once exactextract in pypi is updated again? 🤔

As Butch suggested earlier, maybe we can use the existing commit as the dependency instead for now until the pypi version gets updated. Can you try adding the line below to the dependencies in settings.ini? Assuming that is the right commit. Using # instead of @ may or may not work as well.

git+ssh://git@github.com/isciences/exactextract.git@364acf3ce579c3b51b78e70642460816b4da9ddc
tm-jc-nacpil commented 2 months ago

As Butch suggested earlier, maybe we can use the existing commit as the dependency instead for now until the pypi version gets updated. Can you try adding the line below to the dependencies in settings.ini? Assuming that is the right commit. Using # instead of @ may or may not work as well.

git+ssh://git@github.com/isciences/exactextract.git@364acf3ce579c3b51b78e70642460816b4da9ddc

@tm-danna-ang I added the ff line as recommended here, which uv tries to install but it fails as the wheel hasn't been built. 😞

"exactextract @ git+ssh://git@github.com/isciences/exactextract.git@364acf3ce579c3b51b78e70642460816b4da9ddc"

pasting stacktrace here

error: Failed to prepare distributions
  Caused by: Failed to fetch wheel: exactextract @ git+ssh://git@github.com/isciences/exactextract.git@364acf3ce579c3b51b78e70642460816b4da9ddc
  Caused by: Failed to build: `exactextract @ git+ssh://git@github.com/isciences/exactextract.git@364acf3ce579c3b51b78e70642460816b4da9ddc`
  Caused by: Build backend failed to build wheel through `build_wheel()` with exit status: 1
--- stdout:
*** scikit-build-core 0.9.8 using CMake 3.16.3 (wheel)
*** Configuring CMake...
loading initial cache file /tmp/tmp5fz0yh5u/build/CMakeInit.txt
-- The C compiler identification is GNU 9.4.0
-- The CXX compiler identification is GNU 9.4.0
-- Check for working C compiler: /usr/bin/x86_64-linux-gnu-gcc
-- Check for working C compiler: /usr/bin/x86_64-linux-gnu-gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/x86_64-linux-gnu-g++
-- Check for working CXX compiler: /usr/bin/x86_64-linux-gnu-g++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring incomplete, errors occurred!
See also "/tmp/tmp5fz0yh5u/build/CMakeFiles/CMakeOutput.log".
--- stderr:
CMake Error at CMakeLists.txt:19 (find_package):
  By not providing "FindGEOS.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "GEOS", but
  CMake did not find one.

  Could not find a package configuration file provided by "GEOS" (requested
  version 3.5) with any of the following names:

    GEOSConfig.cmake
    geos-config.cmake

  Add the installation prefix of "GEOS" to CMAKE_PREFIX_PATH or set
  "GEOS_DIR" to a directory containing one of the above files.  If "GEOS"
  provides a separate development package or SDK, be sure it has been
  installed.

*** CMake configuration failed
---
tm-danna-ang commented 2 months ago

@tm-danna-ang I added the ff line as recommended here, which uv tries to install but it fails as the wheel hasn't been built. 😞

oof 😢 then as a temporary workaround maybe replace nodata input (via extra_args) as np.nan prior to the exact_extract() call then revisit once the pypi version is updated

butchtm commented 2 months ago

Notes for post merge tasks: