latent/possible bug with LutView and ArrayView.remove_lut_view

[tlambert03]

with #87, we have a very nice unification of LutViews, with both the GUI object that controls clims and cmap as well as the graphics object on the canvas both being an instance of LutView. ChannelController is capable of managing either of these in it's lut_views list.

However, if one (read: me) is not careful, they might attempt to call ArrayView.remove_lut_view() on all of the lut_views managed by a given ChannelController. However, that would an error currently because only some of those lut_views are "relevant" for the front-end GUI object to "remove". (for example: we don't want to ask a Qt ArrayView to remove a vispy ImageHandle...)

just hit this myself, need to think about best way to handle this.

cc @gselzer if you have any thoughts

[gselzer]

Can we write the API such that ArrayView.remove_lut_view is a no-op if the LutView was not created by the ArrayView? Kinda like using set.discard() over set.remove()...

Creation at removal time could then be determined either by state (i.e. add some _created_views field) or by inheritance (i.e. if it's a QLutView, I must have created it).

[tlambert03]

Can we write the API such that ArrayView.remove_lut_view is a no-op if the LutView was not created by the ArrayView?

Sure, but it's hard to enforce that at an API level: you would just need to make sure that every concrete implementation in the guis check isinstance... (before they had a guarantee, now they all need to check)

[gselzer]

Sure, but it's hard to enforce that at an API level

Right...but it's also not a massive burden, because there are only 3 ArrayViewers right now?

before they had a guarantee, now they all need to check

I'm not sure what you meant by this what was the guarantee?

Maybe you can elaborate on your obverall goal (i.e. why you're doing that necessitates ArrayView.remove_lut_view?

[tlambert03]

no no, it's definitely not hard, and if that's how it needs to be, that's fine... it just has a bit of a smell :). I'm thinking of something that is perhaps a gui specific subclass (like CanvasElement is a graphics specific subclass). I'm looking for a way to ensure via typing that "this thing should never be passed to this method". Currently, it looks like it should be just fine, the method wants a LutView, but if you give it one, it may or may not work.

So, yes, we can implement those three methods and it would definitely work. but it does definitely require additional human-only comments to explain exactly how a subclass should implement it.

[tlambert03]

Maybe you can elaborate on your obverall goal (i.e. why you're doing that necessitates ArrayView.remove_lut_view?

in StreamingViewer, I was cleaning up, and tried for lut_view in ChannelController.lut_views: self._view.remove_lut_view(lut_view). Which seemed reasonable at the time 😂

it's here:

https://github.com/pyapp-kit/ndv/pull/134/files#diff-592e718d87bb42ca2e9ba3e1235442ba4af549406cedc00424386609bd0e3f50R55-R62

[gselzer]

I mean, one other idea is to put remove() on the LutView interface as well...would that get too hairy?

[tlambert03]

yeah, that could also potentially work. as long as each one can find its parent... (which I think it can right?)