mdbartos / pysheds

:earth_americas: Simple and fast watershed delineation in python.
GNU General Public License v3.0
702 stars 188 forks source link

Wrong bbox when using `ViewFinder` #250

Open cheginit opened 3 months ago

cheginit commented 3 months ago

I am using ViewFinder to create a raster directly from an xarray.dataarray, like so:

from pysheds.view import Raster, ViewFinder

viewfinder = ViewFinder(affine=dem.rio.transform(), shape=dem.shape, crs=dem.rio.crs, nodata=dem.rio.nodata)
raster = Raster(dem.to_numpy(), viewfinder=viewfinder)

However, this gets the values of ymin and ymax in bbox incorrectly. This leads to getting an error when checking if a pour point is within the bounds of the data.

Here's the code to reproduce:

from affine import Affine
import numpy as np

affine = Affine(0.9878631881949264, 0.0, 318703.875925, 0, 0.9802706940888642, 273852.257894)
shape = (15936, 21855)
crs = 27700
nodata = np.nan
viewfinder = ViewFinder(affine=affine, shape=shape, crs=crs, nodata=nodata)

With this, for viewfinder.bbox, we get (318703.875925, 289473.8516750001, 340293.6259030001, 273852.257894) while it should be (318703.875925, 273852.257894, 340293.62590300007, 289473.85167500016).

The issue seems to be with the bbox property of ViewFinder:

    @property
    def bbox(self):
        shape = self.shape
        xmin, ymax = View.affine_transform(self.affine, 0, 0)
        xmax, ymin = View.affine_transform(self.affine, shape[-1], shape[-2])
        _bbox = (xmin, ymin, xmax, ymax)
        return _bbox

That I think should be:

    @property
    def bbox(self):
        shape = self.shape
        xmin, ymin = View.affine_transform(self.affine, 0, 0)
        xmax, ymax = View.affine_transform(self.affine, shape[-1], shape[-2])
        _bbox = (xmin, ymin, xmax, ymax)
        return _bbox

For your reference, my affine version is 2.4.0.

mdbartos commented 3 months ago

Thanks for looking into this. Would you be willing to submit a PR?

cheginit commented 3 months ago

For some reason, it's working fine now. I created a fresh env to get ready for a PR and noticed that it's working correctly now. I don't know what was wrong before, since I didn't change anything except for creating a new env. Maybe it has to do with numba caching?

cheginit commented 3 months ago

OK, I figured out the issue using rioxarray's source code. It gets the wrong order depending on the sign of the resolution that you get from the affine. I will open a PR.