#### Question about code fragment in features.geometry_window()

Sean Gillies

Hi,

On Tue, Aug 17, 2021 at 11:35 AM Shinji Suzuki <shinji.suzuki@...> wrote:
Oops, I overlooked the fact that min/max was computed after (x,y) gets
transformed.
Please disregard point 2.

On Wed, Aug 18, 2021 at 2:22 AM Shinji Suzuki via groups.io
<shinji.suzuki=gmail.com@groups.io> wrote:
>
> Hi.
> Starting this line, bounds extended by (pad_x, pad_y) are computed and
> later get transformed.
> https://github.com/mapbox/rasterio/blob/master/rasterio/features.py#L451
>
> What  I don't understand are;
> 1. 'pad_x' is subtracted from 'bottom'. Shouldn't 'pad_y' be the one
> that should be related to 'bottom'?
> 2. Same value is computed twice.(e.g. left - pad_x). Since the
> computed values are used later only for obtaining max and min. There
> are no point in yielding a value multiple times.
>
> If my understanding is correct that this code is for computing the
> bounding box after transformation, should not this be changed as
> follows?
>
> diff --git a/rasterio/features.py b/rasterio/features.py
> index 768f6429..e0764e63 100644
> --- a/rasterio/features.py
> +++ b/rasterio/features.py
> @@ -451,12 +451,12 @@ def geometry_window(
>      xs = [
>          x
>          for (left, bottom, right, top) in all_bounds
> -        for x in (left - pad_x, right + pad_x, right + pad_x, left - pad_x)
> +        for x in (left - pad_x, right + pad_x, right - pad_x, left + pad_x)
>      ]
>      ys = [
>          y
>          for (left, bottom, right, top) in all_bounds
> -        for y in (top + pad_y, top + pad_y, bottom - pad_x, bottom - pad_x)
> +        for y in (top + pad_y, top - pad_y, bottom - pad_y, bottom + pad_y)
>      ]
>
>      rows1, cols1 = rowcol(
>
> Thank you for reading.

Indeed I believe you have found a bug! We're tracking it at https://github.com/mapbox/rasterio/issues/2266.

Thanks,

--
Sean Gillies

Shinji Suzuki

Oops, I overlooked the fact that min/max was computed after (x,y) gets
transformed.
Please disregard point 2.

On Wed, Aug 18, 2021 at 2:22 AM Shinji Suzuki via groups.io
<shinji.suzuki=gmail.com@groups.io> wrote:

Hi.
Starting this line, bounds extended by (pad_x, pad_y) are computed and
later get transformed.
https://github.com/mapbox/rasterio/blob/master/rasterio/features.py#L451

What I don't understand are;
1. 'pad_x' is subtracted from 'bottom'. Shouldn't 'pad_y' be the one
that should be related to 'bottom'?
2. Same value is computed twice.(e.g. left - pad_x). Since the
computed values are used later only for obtaining max and min. There
are no point in yielding a value multiple times.

If my understanding is correct that this code is for computing the
bounding box after transformation, should not this be changed as
follows?

diff --git a/rasterio/features.py b/rasterio/features.py
index 768f6429..e0764e63 100644
--- a/rasterio/features.py
+++ b/rasterio/features.py
@@ -451,12 +451,12 @@ def geometry_window(
xs = [
x
for (left, bottom, right, top) in all_bounds
- for x in (left - pad_x, right + pad_x, right + pad_x, left - pad_x)
+ for x in (left - pad_x, right + pad_x, right - pad_x, left + pad_x)
]
ys = [
y
for (left, bottom, right, top) in all_bounds
- for y in (top + pad_y, top + pad_y, bottom - pad_x, bottom - pad_x)
+ for y in (top + pad_y, top - pad_y, bottom - pad_y, bottom + pad_y)
]

rows1, cols1 = rowcol(

Thank you for reading.

Shinji Suzuki

Hi.
Starting this line, bounds extended by (pad_x, pad_y) are computed and
later get transformed.
https://github.com/mapbox/rasterio/blob/master/rasterio/features.py#L451

What I don't understand are;
1. 'pad_x' is subtracted from 'bottom'. Shouldn't 'pad_y' be the one
that should be related to 'bottom'?
2. Same value is computed twice.(e.g. left - pad_x). Since the
computed values are used later only for obtaining max and min. There
are no point in yielding a value multiple times.

If my understanding is correct that this code is for computing the
bounding box after transformation, should not this be changed as
follows?

diff --git a/rasterio/features.py b/rasterio/features.py
index 768f6429..e0764e63 100644
--- a/rasterio/features.py
+++ b/rasterio/features.py
@@ -451,12 +451,12 @@ def geometry_window(
xs = [
x
for (left, bottom, right, top) in all_bounds
- for x in (left - pad_x, right + pad_x, right + pad_x, left - pad_x)
+ for x in (left - pad_x, right + pad_x, right - pad_x, left + pad_x)
]
ys = [
y
for (left, bottom, right, top) in all_bounds
- for y in (top + pad_y, top + pad_y, bottom - pad_x, bottom - pad_x)
+ for y in (top + pad_y, top - pad_y, bottom - pad_y, bottom + pad_y)
]

rows1, cols1 = rowcol(

Thank you for reading.

 1 - 3 of 3