Skip to content

Comments

Set INIT_DEST to 0 instead of NO_DATA if it's unset#3389

Merged
snowman2 merged 1 commit intorasterio:mainfrom
QuLogic:init-dest-0
Sep 8, 2025
Merged

Set INIT_DEST to 0 instead of NO_DATA if it's unset#3389
snowman2 merged 1 commit intorasterio:mainfrom
QuLogic:init-dest-0

Conversation

@QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Sep 2, 2025

This matches behaviour in GDAL < 3.11 and intended behaviour in 3.11 (which was supposed to warn about this, but still actually fails; see OSGeo/gdal#12198 (review).)

This implements the suggestion from OSGeo/gdal#12189 (comment)

@QuLogic
Copy link
Contributor Author

QuLogic commented Sep 2, 2025

Test failures are due to click 8.2.1: pallets/click#2939

if init_dest_nodata:
warp_extras = CSLSetNameValue(warp_extras, "INIT_DEST", "NO_DATA")
warp_extras = CSLSetNameValue(warp_extras, "INIT_DEST",
"NO_DATA" if dst_nodata is not None else "0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the default be NaN for floating point bands, or is the intention to initialize with something that looks like a legal value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aim was to replicate what GDAL used to, and will soon revert back to, doing, before that warning becomes something permanent.

There may be more precise values that could be applied instead, but this is the minimal fix to the test failures, and isn't a behaviour change that might require more complicated discussion/review/API notes/etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, getting a new Rasterio release out is more important than refining these things.

@w8sl
Copy link
Contributor

w8sl commented Sep 5, 2025

Test failures are due to click 8.2.1: pallets/click#2939

Click 8.2.2 has been withdrawn from PyPI.

The temporary solution may be to just pin

click==8.1.8

in:

requirements.txt
setup.py

Few remaining test failures may need something like:

def test_warpedvrt_gcps__width_height(tmp_path):
    tiffname = tmp_path / "test.tif"
    src_gcps = [
        GroundControlPoint(row=0, col=0, x=156113, y=2818720, z=0),
        GroundControlPoint(row=0, col=800, x=338353, y=2785790, z=0),
        GroundControlPoint(row=800, col=800, x=297939, y=2618518, z=0),
        GroundControlPoint(row=800, col=0, x=115698, y=2651448, z=0),
    ]
    crs = CRS.from_epsg(32618)
    with rasterio.open(tiffname, mode='w', height=800, width=800, count=3, dtype=numpy.uint8) as source:
        source.gcps = (src_gcps, crs)

    with rasterio.open(tiffname) as src:
        # GDAL 3.10 and earlier used a half‐pixel center offset        
        # GDAL 3.11+ uses corner‐based origin: c == minX
        expected_transforms = [
           affine.Affine(22271.389322449897, 0.0, 115698.0, 0.0, -20016.05875815117, 2818720.0),
           affine.Affine(22271.389322449897, 0.0, 115698.25, 0.0, -20016.05875815117, 2818720.0)
           ]
        with WarpedVRT(src, width=10, height=10) as vrt:
            assert vrt.height == 10
            assert vrt.width == 10
            assert vrt.crs == crs
            assert any(vrt.dst_transform.almost_equals(t) for t in expected_transforms)

@QuLogic
Copy link
Contributor Author

QuLogic commented Sep 7, 2025

The temporary solution may be to just pin

Seems enough: #3393. I've also rebased this on top of it.

This matches behaviour in GDAL < 3.11 and _intended_ behaviour in 3.11
(which was supposed to warn about this, but still actually fails.)
Comment on lines 584 to +586
if init_dest_nodata:
warp_extras = CSLSetNameValue(warp_extras, "INIT_DEST", "NO_DATA")
warp_extras = CSLSetNameValue(warp_extras, "INIT_DEST",
"NO_DATA" if dst_nodata is not None else "0")
Copy link
Member

@snowman2 snowman2 Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work?

Suggested change
if init_dest_nodata:
warp_extras = CSLSetNameValue(warp_extras, "INIT_DEST", "NO_DATA")
warp_extras = CSLSetNameValue(warp_extras, "INIT_DEST",
"NO_DATA" if dst_nodata is not None else "0")
if init_dest_nodata and dst_nodata is not None:
warp_extras = CSLSetNameValue(warp_extras, "INIT_DEST", "NO_DATA")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, then it's not initialized at all and it breaks a lot of tests.

Comment on lines 1022 to +1023
if init_dest_nodata is True and 'init_dest' not in warp_extras:
self.warp_extras['init_dest'] = 'NO_DATA'
self.warp_extras['init_dest'] = 'NO_DATA' if self.dst_nodata is not None else '0'
Copy link
Member

@snowman2 snowman2 Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work?

Suggested change
if init_dest_nodata is True and 'init_dest' not in warp_extras:
self.warp_extras['init_dest'] = 'NO_DATA'
self.warp_extras['init_dest'] = 'NO_DATA' if self.dst_nodata is not None else '0'
if init_dest_nodata is True and self.dst_nodata is not None and 'init_dest' not in warp_extras:
self.warp_extras['init_dest'] = 'NO_DATA'

Copy link
Contributor Author

@QuLogic QuLogic Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, see above (though for fewer tests, I think.)

@snowman2
Copy link
Member

snowman2 commented Sep 8, 2025

Thanks @QuLogic 👍

@snowman2 snowman2 merged commit 347a97e into rasterio:main Sep 8, 2025
21 of 22 checks passed
@QuLogic QuLogic deleted the init-dest-0 branch September 9, 2025 00:20
@snowman2 snowman2 added this to the 1.4.4 milestone Sep 11, 2025
@snowman2 snowman2 added the backport-1.4 To be backported to maint-1.4 label Nov 6, 2025
snowman2 pushed a commit that referenced this pull request Nov 6, 2025
This matches behaviour in GDAL < 3.11 and _intended_ behaviour in 3.11
(which was supposed to warn about this, but still actually fails.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-1.4 To be backported to maint-1.4 bug upstream

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants