Add stack_labels property to WeightedStackColorMapper

[ianthomas23]

This adds a stack_labels property to WeightedStackColorMapper, fixing #13200. It is not used for anything yet but will be used to provide richer hover tooltips for ImageStack glyphs as described in issue #13354 that will follow in another PR.

Example (though not very interesting until the tooltips have been implemented):

from bokeh.models import EqHistColorMapper, WeightedStackColorMapper
from bokeh.palettes import varying_alpha_palette
from bokeh.plotting import figure, show
import numpy as np

stack = np.asarray([[[0, 1], [1, 1], [0, 1]],
                    [[1, 0], [2, 2], [3, 3]],
                    [[0, 1], [4, 1], [3, 5]]])

p = figure(width=400, height=300)
alpha_mapper = EqHistColorMapper(
    palette=varying_alpha_palette(color="#000", n=10, start_alpha=40),
    rescale_discrete_levels=False,
)
color_mapper = WeightedStackColorMapper(
    palette=["yellow", "purple"],
    nan_color=(0, 0, 0, 0),
    alpha_mapper=alpha_mapper,
    stack_labels=["some", "other"],
)

im = p.image_stack(image=[stack], x=0, y=0, dw=1, dh=1, color_mapper=color_mapper)
p.add_layout(im.construct_color_bar(), "right")
show(p)

The stack_labels is a sequence (list or tuple) of strings and must be the same length as the palette. If not an error is produced and the stack_labels are altered to be the correct length in the spirit of defensive programming. It could just be a warning rather than an error.

In the original issue (#13200) I considered this approach but favoured adding the property to ImageStack glyph. I have changed my mind as this here is simpler and more logical as the labels refer to the palette colors so should be set in the same place, and it is easy to enforce their lengths being identical.

• issues: fixes [FEATURE] Preserve category information for categorical colormapping #13200
• tests added / passed
• release document entry (if new feature or API change)

(Edited to change property name)

[bryevdv]

I have changed my mind as this here is simpler and more logical as the labels refer to the palette colors so should be set in the same place, and it is easy to enforce their lengths being identical.

I think this seems fine, but would it make sense on the base StackColorMapper instead? IIUC correctly then image_stack really always requires a StackColorMapper of some sort? In which case we might want labels to work with any one them. FMI is there anything currently that mandates a color mapper be configured for image_stack?

[codecov]

Codecov Report

Merging #13366 (771934d) into branch-3.3 (3348f65) will increase coverage by 0.06%.
Report is 6 commits behind head on branch-3.3.
The diff coverage is 100.00%.

@@              Coverage Diff               @@
##           branch-3.3   #13366      +/-   ##
==============================================
+ Coverage       92.42%   92.48%   +0.06%     
==============================================
  Files             315      315              
  Lines           20169    20198      +29     
==============================================
+ Hits            18641    18680      +39     
+ Misses           1528     1518      -10     

📢 Have feedback on the report? Share it here.

[ianthomas23]

IIUC correctly then image_stack really always requires a StackColorMapper of some sort? In which case we might want labels to work with any one them. FMI is there anything currently that mandates a color mapper be configured for image_stack?

ImageStack does require a StackColorMapper but it is is not enforced. PR #13209 is moving in this direction, but really we need a richer validation system to avoid that being the bunch of workarounds that it is currently.

I think this seems fine, but would it make sense on the base StackColorMapper instead?

Maybe?!?

I created WIP PR #13316 to add an IndexedStackColorMapper so that we have another use of ImageStack to just colormap/display a single 2D image from a stack of them. The idea is to have another possible use of ImageStack around so that we don't make design decisions purely based on the WeightedStackColorMapper (aka categorical Datashader) use case.

Thinking about this further:

• For WeightedStackColorMapper, len(label) == len(palette) and also len(label) == im.shape[2] for each im in ImageStack.image.
• For IndexedStackColorMapper, len(label) == im.shape[2] for each im in ImageStack.image and len(palette) is not relevant.

So we could push label up to StackColorMapper and for validation use the common test against im.shape[2] for each of the im. The disadvantage here is that this validation cannot occur within the StackColorMapper but only within ImageStack, so we'd have to do something hacky like #13209 (which was never intended to be a serious PR).

[bryevdv]

I see, that's more complex than I had appreciated. I wonder would it be better to keep things on WeightedStackColorMapper but adopt a less generic name, level_labels? I'm not sure what the best terminology here would be, but I'm imagining on IndexedStackColorMapper it could be index_labels, say. Basically: if they aren't the same thing really, let's also make it easy to talk about them separately.

[ianthomas23]

How about stack_labels for the name? The docstring for WeightedStackColorMapper begins with "Maps 3D data arrays of shape (ny, nx, nstack) to 2D RGBA images of shape (ny, nx) using a palette of length nstack" so we can say that len(stack_labels) == nstack.

In fact maybe this helps us in the generic case as if we ever need this in IndexedStackColorMapper we can push stack_labels up to StackColorMapper and still keep the len(stack_labels) == nstack explanation. WeightedStackColorMapper has the extra condition that nstack == len(palette) but this is really a condition on the palette rather than the stack_labels. This is saying that the stack_labels are the same thing for weighted and indexed, which I think might be a better way of explaining them (when I've updated the docs).

[ianthomas23]

I have changed the property name to stack_labels but I am open to other names if anyone has a strong opinion.