Skip to content

Comments

Warping: fix inconsistent replacement of valid value that collides with the dstnodata value#11713

Merged
rouault merged 2 commits intoOSGeo:masterfrom
rouault:fix_11711
Jan 26, 2025
Merged

Warping: fix inconsistent replacement of valid value that collides with the dstnodata value#11713
rouault merged 2 commits intoOSGeo:masterfrom
rouault:fix_11711

Conversation

@rouault
Copy link
Member

@rouault rouault commented Jan 24, 2025

Fixes #11711

While technically a bug fix, I'm somewhat 50% 50% to backport that to 3.10, in particular as it affects probably quite common use cases like Byte nearest resampling. Thoughts @dbaston ?

@rouault rouault added the funded through GSP Work funded through the GDAL Sponsorship Program label Jan 24, 2025
@vincentschut
Copy link
Contributor

vincentschut commented Jan 24, 2025

as the original bug issuer of #11711 : we don't need a backport, we have fixed our code internally to avoid hitting this corner case. But anyway thanks for the quick follow-up!

@dbaston
Copy link
Member

dbaston commented Jan 24, 2025

I wouldn't be inclined to backport it because it changes behavior in a way that might not be expected. Since there is no unambiguously correct behavior in this situation I think it would be good to emit a one-time warning like ("value %d in the source dataset has been changed to %d in the destination dataset to avoid being treated as NoData. To avoid this, select a different NoData value for the destination dataset.")

@coveralls
Copy link
Collaborator

coveralls commented Jan 24, 2025

Coverage Status

coverage: 70.061% (+0.002%) from 70.059%
when pulling 2b0b1c5 on rouault:fix_11711
into a9854d1 on OSGeo:master.

@rouault
Copy link
Member Author

rouault commented Jan 24, 2025

I think it would be good to emit a one-time warning like ("value %d in the source dataset has been changed to %d in the destination dataset to avoid being treated as NoData. To avoid this, select a different NoData value for the destination dataset.")

done

@rouault rouault merged commit 6074ce4 into OSGeo:master Jan 26, 2025
38 checks passed
@schwehr
Copy link
Member

schwehr commented Jan 27, 2025

@sgillies @rouault Another rasterio failure from a warp change with 6074ce4

assert 1314520 == 1309625
 +  where 1314520 = <function count_nonzero at 0x3217fdd4b2b0>(array([14, 17, 11, ..., 42, 43, 45], shape=(1315638,), dtype=uint8))
 +    where <function count_nonzero at 0x3217fdd4b2b0> = np.count_nonzero
test3d = True, count_nonzero = 1309625
path_rgb_byte_tif = 'data/RGB.byte.tif'

    @pytest.mark.parametrize("test3d,count_nonzero", [(True, 1309625), (False, 437686)])
    def test_reproject_array_interface(test3d, count_nonzero, path_rgb_byte_tif):
        class DataArray:
            def __init__(self, data):
                self.data = data
    
            def __array__(self, dtype=None):
                return self.data
    
            @property
            def dtype(self):
                return self.data.dtype
    
        with rasterio.open(path_rgb_byte_tif) as src:
            if test3d:
                source = DataArray(src.read())
            else:
                source = DataArray(src.read(1))
        out = DataArray(np.empty(source.data.shape, dtype=np.uint8))
        reproject(
            source,
            out,
            src_transform=src.transform,
            src_crs=src.crs,
            src_nodata=src.nodata,
            dst_transform=DST_TRANSFORM,
            dst_crs="EPSG:3857",
            dst_nodata=99,
        )
        assert isinstance(out, DataArray)
>       assert np.count_nonzero(out.data[out.data != 99]) == count_nonzero
E       assert 1314520 == 1309625
E        +  where 1314520 = <function count_nonzero at 0x3217fdd4b2b0>(array([14, 17, 11, ..., 42, 43, 45], shape=(1315638,), dtype=uint8))
E        +    where <function count_nonzero at 0x3217fdd4b2b0> = np.count_nonzero

@rouault
Copy link
Member Author

rouault commented Jan 27, 2025

@schwehr That's likely expected and should be filed on rasterio side.

@schwehr
Copy link
Member

schwehr commented Jan 28, 2025

This commit triggered a test failure with the Earth Engine Data Catalog SMAP processing. Here is the fix:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

funded through GSP Work funded through the GDAL Sponsorship Program

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gdal warp changes 0's into 1's when called with a specific combination of options

5 participants