Eliaaazzz opened a new pull request, #3327: URL: https://github.com/apache/sedona/pull/3327
## Did you read the Contributor Guide? - Yes, I have read the [Contributor Rules](https://sedona.apache.org/latest/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/latest/community/develop/) ## Is this PR related to a ticket? - Yes, and the PR name follows the format `[GH-XXX] my subject`. Closes #3324 > **Stacked on #3312.** #3324 is a follow-up to that PR and only reproduces with it applied, so this branch starts from `2b06236` (its current head) and adds two commits on top. The first three commits in the diff belong to #3312. Please merge #3312 first and I will rebase this onto `master`, or fold this into #3312 if that is easier to review. ## What changes were proposed in this PR? Two independent commits, so the second can be dropped without losing the first. ### 1. Keep other bands' no-data value when clearing one band (`309b16a`) No-data values are tracked per band on the sample dimensions. `GC_NODATA` is a single sentinel for the whole coverage, carried both by the coverage properties and by the rendered image. The clear path dropped that sentinel unconditionally, so clearing one band also stopped GeoTools operations that read the sentinel from honouring the bands that still declared a no-data value. Jiffle reads it, which is how clearing band 2 changed band 1's `out = rast[0] + 1;` from `NaN` to `1.0`. The fix drops the sentinel only once no band declares a no-data value at all. Reproduced before the change and verified after, on the exact steps in the issue: - Loaded from GeoTIFF: band 1 `0.0`, band 2 `0.0`, `rast[0] + 1` gives `NaN`. - After clearing band 2, before this PR: band 1 `0.0`, band 2 `null`, `rast[0] + 1` gives `1.0`. This is the bug. - After clearing band 2, with this PR: band 1 `0.0`, band 2 `null`, `rast[0] + 1` gives `NaN`. - After clearing band 1 as well: band 1 `null`, band 2 `null`, `rast[0] + 1` gives `1.0`. ### 2. Reject mixed per-band no-data values when writing GeoTiff (`9e5525c`) This is the second half of the issue: a `(0, null)` raster came back from a GeoTIFF round trip as `(0, 0)`. GDAL_NODATA is one value for the whole file, so that state cannot be written faithfully. `RS_AsGeoTiff` now throws `IllegalArgumentException` on it, rather than resurrecting a no-data value on a band that had it cleared. **This commit needs a maintainer decision, because its blast radius is wider than the clear path.** `RasterUtils.copyRasterAndAppendBand` sets the appended band's no-data value and leaves the existing bands' sample dimensions alone, so `RS_AddBand` and `RS_Union` routinely produce rasters whose bands disagree. Writing one of those to GeoTIFF silently rewrites the other bands' no-data today; with this commit it fails loudly. That follows the issue's preference for rejecting unsupported mixed states, but if it reads as too strong for `RS_AsGeoTiff`, dropping `9e5525c` leaves commit 1 intact and I will open a separate issue for the writer. ## How was this patch tested? New tests in `common`: - `RasterBandEditorsTest.testSetBandNoDataValueWithNullKeepsNoDataOnOtherBands` covers the issue's reproducer. It checks both bands and map algebra after clearing either one, before serialization, and that the input raster is not mutated. - `RasterBandEditorsTest.testSetBandNoDataValueWithNullOnEveryBandClearsNoDataProperty` covers the case where every band is cleared: the sentinel is dropped from the coverage and the image, map algebra reads the zeros as ordinary data, and the state survives a GeoTIFF round trip. - `RasterOutputTest.testAsGeoTiffRejectsBandsWithDifferentNoDataValues` covers "only one band set" and "both set but different". Rasters whose bands agree, and rasters with no no-data value at all, still round-trip. `mvn -pl common test` was run before and after the change on Windows with JDK 17. The same 13 tests fail both times: `RasterBandEditorsTest.testClip`, 5 `RasterEditorsTest.testResample*`, and 7 `RasterOutputTest.testAsMatrix*`. They fail identically on unmodified `2b06236`, so they are pre-existing on this platform and unrelated to this change. Everything else passes, including the three new tests. I could not run `spark/common` locally. It fails to compile on the generated OSM PBF protobuf sources (`package proto4 does not exist`) before reaching any raster code, so those suites are left to CI. One note for whoever reviews the tests. `MapAlgebra` caches a compiled Jiffle runtime per script text in a `ThreadLocal`, so I checked whether a cached runtime could mask this regression by carrying a previous raster's no-data handling into a later call with the same script. It does not: `setSourceImage` re-reads the property. The tests therefore use a plain shared script. ## Did this PR include necessary documentation updates? - Yes, I have updated the documentation. `RS_SetBandNoDataValue` now states that no-data is tracked per band, and `RS_AsGeoTiff` documents the single-value-per-file constraint and the new rejection. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01P6hpqAnXfeaWRLkL1VQDMd -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
