Set INIT_DEST to 0 instead of NO_DATA if it's unset#3389
Set INIT_DEST to 0 instead of NO_DATA if it's unset#3389snowman2 merged 1 commit intorasterio:mainfrom
Conversation
|
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") |
There was a problem hiding this comment.
Should the default be NaN for floating point bands, or is the intention to initialize with something that looks like a legal value?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I agree, getting a new Rasterio release out is more important than refining these things.
Click 8.2.2 has been withdrawn from PyPI. The temporary solution may be to just pin in: Few remaining test failures may need something like: |
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.)
| 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") |
There was a problem hiding this comment.
Does this work?
| 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") |
There was a problem hiding this comment.
No, then it's not initialized at all and it breaks a lot of tests.
| 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' |
There was a problem hiding this comment.
Does this work?
| 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' |
There was a problem hiding this comment.
No, see above (though for fewer tests, I think.)
|
Thanks @QuLogic 👍 |
This matches behaviour in GDAL < 3.11 and _intended_ behaviour in 3.11 (which was supposed to warn about this, but still actually fails.)
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)