[GH-3330] Keep the other bands when replacing a band's no-data value - #3331
Open
Eliaaazzz wants to merge 1 commit into
Open
[GH-3330] Keep the other bands when replacing a band's no-data value#3331Eliaaazzz wants to merge 1 commit into
Eliaaazzz wants to merge 1 commit into
Conversation
The replacement allocates a fresh WritableRaster with every band and then wrote only the target band into it, so the remaining bands came back as the allocation default of 0 while their sample dimensions still described the original data. On a three-band raster, setBandNoDataValue(raster, 1, -999, true) returned bands 2 and 3 filled with zeros. Copy every band across, replacing the old no-data value only in the targeted one. Both existing tests for the replace option use single-band rasters, which is why this went unnoticed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Did you read the Contributor Guide?
Is this PR related to a ticket?
[GH-XXX] my subject. Closes RS_SetBandNoDataValue with replace=true zeroes every band except the target one #3330What changes were proposed in this PR?
RasterBandEditors.setBandNoDataValue(raster, bandIndex, noDataValue, replace = true)allocates a freshWritableRasterwith every band and then wrote onlybandIndex - 1into it. The remaining bands came back as the allocation default of0, while their sample dimensions still described the original data, so the raster looked intact in its metadata and had lost its pixels.On a three-band raster,
setBandNoDataValue(raster, 1, -999.0, true)returned:[1, 2, 3, 4, -999, 6, 7, 8, 9], correct[11 ... 19]was expected[21 ... 29]was expectedReplacing on a band other than the first lost band 1 the same way. The fix copies every band across and applies the replacement only to the targeted one.
This reaches SQL as
RS_SetBandNoDataValue(raster, bandIndex, noDataValue, true)on any raster with two or more bands.How was this patch tested?
New test
RasterBandEditorsTest.testSetBandNoDataValueWithReplaceOptionKeepsOtherBands, which replaces on band 1 and then on band 2 of a three-band raster and asserts the full contents of all three bands each time, plus the resulting no-data metadata. It fails onmasterwith band 2 and band 3 read back as zeros.Both existing tests for the replace option,
testSetBandNoDataValueWithReplaceOptionandtestSetBandNoDataValueWithReplaceOptionRaster, use single-band rasters, which is why this went unnoticed. They still pass unchanged.mvn -pl common teston Windows with JDK 17: the same 13 tests fail before and after the change (RasterBandEditorsTest.testClip, 5RasterEditorsTest.testResample*, 7RasterOutputTest.testAsMatrix*). They fail identically on unmodifiedmaster, so they are pre-existing on this platform and unrelated. Everything else passes.I could not run
spark/commonlocally: it fails to compile on the generated OSM PBF protobuf sources (package proto4 does not exist) before reaching any raster code. Leaving those suites to CI.Did this PR include necessary documentation updates?
RS_SetBandNoDataValuealready documents.🤖 Generated with Claude Code
https://claude.ai/code/session_01P6hpqAnXfeaWRLkL1VQDMd