fill value not respected when saving palette images while keeping palette

[gerritholl]

Describe the bug

When saving a palette image (mode P) with keep_palette=True, XRImage._replace_fill_value does not get called and fill values are not applied.

To Reproduce

import numpy as np
import xarray as xr
import trollimage.xrimage
import rasterio

arr = np.ones((1, 5, 5), dtype="uint8")
arr[0, 2, 2] = 255
data = xr.DataArray(
        arr, dims=["bands", 'y', 'x'],
        attrs={"_FillValue": 255},
        coords={"bands": ["P"]})
img = trollimage.xrimage.XRImage(data)
img.save("test.tif", keep_palette=True, fill_value=42)
f = rasterio.open("test.tif")
cont = f.read()
print(cont)

Expected behavior

I expect that the fill value is replaced, like it is when keep_palette=False (the default).

[[[  1   1   1   1   1]
  [  1   1   1   1   1]
  [  1   1  42   1   1]
  [  1   1   1   1   1]
  [  1   1   1   1   1]]]

Actual results

In reality, the fill value is ignored:

[[[  1   1   1   1   1]
  [  1   1   1   1   1]
  [  1   1 255   1   1]
  [  1   1   1   1   1]
  [  1   1   1   1   1]]]

Environment Info:

• trollimage Version: main

Additional context

It's explicitly excluded:

trollimage/trollimage/xrimage.py

Lines 911 to 912 in f0d8d4c

	if not keep_palette:
	final_data = self._scale_alpha_or_fill_data(final_data, fill_value, dtype)

The exclusion was there ever since keep_palette was introduced in #39.

[djhoese]

I think we need to be really careful with the solution for this. As you know, palettize is different from the other enhancement methods/functions as the resulting values are indexes into a color table/map. So I think there is an assumption that every value in the result is a valid index in the colormap. In both the current result and expected result I could see this being hard to ensure. Also, should the fill value be replaced as an index (as you've described it) or should it be done before the palettizing?

[gerritholl]

Every value is a valid index in the colormap, but the colormap contains no information about the interpretation of the colors (legend / palette meanings). We can define the color with index 0 to be used for missing data, or the color with index 255, or another value, but this information is not contained in the color table itself (at most implicitly via the alpha value, but pixels mapping to transparency are not necessarily invalid data).

In the current result, we force the user to use the color with index 255 for missing data. My user (NinJo) has asked to use the color with index 0 for missing data. This suits well for nwcsaf-geo cloud_top_height, where the colour for pixels with value 0 has alpha 0 (fully transparent), whereas the colour for pixel value 255 is black.

Hence this issue and the sister issue #178. The NWCSAF-GEO CTH data use a pixel value of zero for missing data, but there is currently no way to encode this information in the output file for a mode P GeoTIFF image.