[nvaytet]

We turn the LokiBankViewer into a more generic tool that could also be used for other instruments.

• You can choose what dimensions are displayed using the show_dims arg.
• You can add horizontal or vertical masks by activating a masking tool
• zooming and panning are now available on the single bank panels

[nvaytet]

This replaces coordinates for every dimension, including existing calibrated coordinates and units, so the plotted axes can become misleading. Could this add coordinates only for dimensions that lack them? If existing coordinates are preserved, mask-click positions will also need converting to pixel indices.

[nvaytet]

@SimonHeybrock in the latest update, the masks are added with only the dimensions they require.
So a mask for an entire tube will have dimensions (layer, tube) instead of all 4 dimensions as it was before.
Which dimensions to give the mask is determined by which dimensions are displayed and the state of the slider sum checkboxes.

[SimonHeybrock]

The mask name is the set of dimensions, so the number of stored masks depends on which dimensions a stroke covers, not on what the user wants to keep apart.

• There is no way to keep, say, a beam-stop mask and a dead-tube mask separate and apply or drop them independently.
• There is also no way of having a mask that does not mask anything. I think this is relevant since there is a difference between "we did not apply the dead-tubes mask" and "we applied the dead-tubes mask (but currently no tube is dead)".
• We may want to open the tool with existing masks that should be edited.

I feel like giving an explicit dict with named masks (or just using the da.masks of the input) would avoid this and related problems. We should have a chat about this.

Comments on current solution from Claude below, but those may not be fully relevant, depending on what we conclude above.

---

Clearing and saving to a second file does separate them, but both files use the same names, so applying both loses the first:

da.assign_masks(file_a['bank_0']).assign_masks(file_b['bank_0'])  # file_a's mask is replaced

If separate masks are wanted, one option is a name field in MaskControl, used as a prefix so that strokes with different dims can still coexist under one name:

mask_name = f'{self.mask_control.mask_name}{dims}'

Two related questions:

• The constructor cannot take existing masks, so a saved file cannot be reopened and edited. Is that intended?
• What will read these files? The existing PixelMaskFilename paths read XML detector-ID lists (SANS) or an HDF5 variable of detector IDs (powder), and name each mask after its file. Neither reads this {bank: {dims: mask}} layout.

[nvaytet]

The constructor cannot take existing masks, so a saved file cannot be reopened and edited. Is that intended?

This was planned for a follow-up, so as to not make this any larger than it already is. But I can add it here if deemed critical.

[SimonHeybrock]

If we don't do it now, wouldn't it require either keeping two parallel mechanism, or remove the current one?

[SimonHeybrock]

The toggle only looks at the mask for the current dims. If a column was masked with layer summed (mask dims ('x',)), clicking the same column with the layer slider active does not unmask it. It adds a second mask ('layer', 'x'), and the column stays masked. Only Clear removes the first one. If this is intended, could the docstring say so?

[nvaytet]

That is how it works currently, The behaviour was slightly more intuitive in the first iteration where the second click would unmask only the visible part of the mask that spans the full dimension.

When it adds the second mask, it feels as if nothing happened. Clicking again removes the second mask, once again feeling like nothing happened because the mask with dim x is still masking all layers.

I did not have a good solution to this, if we want to keep a dict of masks that can all have different dimensions.
We don't want to broadcast the existing 'x' mask to then unmask only the first layer of it.

The alternative would be to just remove any mask that is hit by the click. But this is again problematic if you made a horizontal mask, and then switch to vertical masks. When you click on a masked horizontal stripe, should it add a vertical mask or should it unmask the existing horizontal mask?

Or maybe we should not toggle on click? Instead, the user need to enable "Delete mode" with the toggle buttons and only then do clicks delete visible masks and don't add new ones. The toggling was just a shortcut to make it efficient to mask/unmask without having to go back to the toggle buttons constantly.
We could still have an additional "Clear" button that would clear all visible masks in one go.

[SimonHeybrock]

event.xdata is in coordinate units but is used as an index below. _add_missing_coordinates keeps existing dimension coordinates, so a bank that has one gets the wrong pixel or an error:

# x = [0.1, 0.2, 0.3, 0.4] m, click on x=0.3
round(0.3) == 0  # masks column 0, no error
# x = [1000, ..., 1003], click on x=1003
# IndexError: The requested index 1003 is out of range

The click position needs converting to an index using the coordinate. A test with a non-arange coordinate would cover this.

[SimonHeybrock]

event.inaxes is also set when the pointer is over the colorbar, so this check does not exclude it. With a masking mode active, a click low on the colorbar silently masks a pixel, because x and y come from the colorbar's data coordinates. A click higher up raises IndexError. Dragging from the image onto the colorbar in continuous mode does the same. Comparing with the image axes here and in add_mask fixes both:

if event.inaxes is not self.tab_figs[self.current_key].ax:
    return

[SimonHeybrock]

When dim is absent from a bank, the slider starts with sum=True but value is (dim, 0), because _on_subwidget_change has not run. add_mask then keeps it in slices, and m[('layer', 0)] raises DimensionError on every masking click in that bank. Reproduced with banks of dims (layer, y, x) and (y, x).

self.value = (self.dim, None if disabled else self.slider.value)

[SimonHeybrock]

The Text fields update on every keystroke, so typing 1e3 or -5 runs this on the partial text and raises ValueError. continuous_update=False on vmin and vmax handles partial input. A final invalid value would still raise, so this needs handling too.

[SimonHeybrock]

DetectorExplorer fails with plopp 26.7 (AxesImage.set() got an unexpected keyword argument 'hide_log_buttons'), but only the docs extra has the >=26.10.0 bound. Could test have it too?

[SimonHeybrock]

nit: ydim, xdim = self._visible_dims gives the same result without reading plopp's private view._dims, since the plotted data is transposed to that order.

[SimonHeybrock]

visible_dims is stored but never read in DimSlider, and _lock is unused here and in ColorControl. Can these be removed?

[SimonHeybrock]

data also accepts a single DataArray (see the hasattr branch below), which the annotation and docstring do not mention. figsize has no annotation.