VeinsOfTheEarth / rabpro

Delineating watershed basins and computing attribute statistics using Google Earth Engine
https://VeinsOfTheEarth.github.io/rabpro
BSD 3-Clause "New" or "Revised" License
35 stars 8 forks source link

pin to shapely < 2.0 #150

Closed jonschwenk closed 1 year ago

jonschwenk commented 1 year ago

I'm seeing reports of errors raised by shapely by users that are related to the new way that shapely accesses MultiPolygon objects. The real fix here would be to update rabpro's codebase to handle both versions, but the temporary fix is to just pin the shapely dependency to less than version 2.0. Can @jsta or @tzussman do this and test to make sure it installs, then repackage rabpro for conda/pip installers?

jsta commented 1 year ago

Any idea how to run the tests on both shapely < 2 and shapely >= 2 in ghactions?

tzussman commented 1 year ago

Not sure if there's a formalized way to do this in GH actions, but you can specify a shapely version as part of a matrix strategy - this will run a job for each value in the matrix with a variable set to the value for that job.

Then, after setting up conda/mamba/pip/whatever environment manager the action uses and installing all the dependencies, you could add another step to explicitly install the shapely version as specified by the matrix variable.

I haven't tested this and I'm not 100% sure this would work that well though... Python dependency management + installation is painful enough as-is without introducing very specific version installations after everything else has already been set up. This may very well end up causing environment installation breakages in the future if package dependencies go wild

jsta commented 1 year ago

seems like other people are creating wholely separate environment files and iterating over a matrix of them

jsta commented 1 year ago

@tzussman I just want an easy non-local way to see the errors when shapely is >= 2 and maybe eventually do the handling that Jon describes

jsta commented 1 year ago

Tests currently pass with shapely >= 2.0.0 (locally). I would prefer to have a test that fails with shapely >= 2.0.0 before pinning below that.

I've successfully triggered multipolygon operations in https://github.com/VeinsOfTheEarth/rabpro/blob/main/rabpro/basins.py#L119 with the following code:

# Russia dateline crossing
rpo = rabpro.profiler(("65.384", "-179.862"), da=3234)
rpo.delineate_basin(force_merit=True)
rpo.watershed.to_file("test.gpkg", driver="GPKG")
rpo.elev_profile(dist_to_walk_km=5)
rpo.export("all")

but still no error.

jsta commented 1 year ago

Closing for now. We can re-open when/if we get a bug report on shapely >= 2.0.0