The _plopp_mask on line objects needs to be updated after data update
#349
Conversation
| @@ -192,11 +192,13 @@ def update(self, new_values: sc.DataArray): | |||
| """ | |||
| self._data = new_values | |||
| new_values = make_line_data(data=self._data, dim=self._dim) | |||
| line_mask = ~np.isnan(new_values['mask']['y']) | |||
There was a problem hiding this comment.
It is unclear why this is needed, and not any of the other things the _make_line method above does. The code is duplicate. What if one of them is changed? __init__ seems to be calling _make_line on _make_line_data... maybe something can/should be moved around?
There was a problem hiding this comment.
The _plopp_mask is needed because the canvas is in charge of autoscaling the axes (this was just a choice, it does not necessarily have be this way). However, it does not know about the Plopp lines or images that are shown onto it, it just handles the axes.
To perform auto-scaling, it looks at the matplotlib artists it holds, but needs to handle lines made by plopp in a special way , as they come with an (most of the time) invisible line of masks (which is why we can't just let the plotting library do the work, unfortunately, also it is bad at handling log axes, and we do better there).
All this feels (and has felt for a long time) a bit weird: the canvas doesn't know about the lines or images, but actually it does because it has to do the auto-scaling. Maybe the auto-scaling should be done by someone else who does have knowledge of the artists. Maybe each Line and Image should provide what its bounding box is (taking into account its own masks etc), and the auto-scaling should just be a simple loop over artists and merge all the bounding boxes.
__init__seems to be calling_make_lineon_make_line_data... maybe something can/should be moved around?
Yes, I don't like this either, and can't really remember why the _make_line function exists.
I have plans to rework all the internals of how the plotting libraries hook in as backends, as I feel there is an awful lot of code that is duplicated between different plot types etc (the idea is kind of drafted here: mesh-3d...simpler-backends).
All this means that it may take a while before all these changes make it in. The issue I found here is a bug that makes the rectangle selection example in the gallery very unusable.
Should we just get these changes in, make a patch release and then work on the large changes?
There was a problem hiding this comment.
Should we just get these changes in, make a patch release and then work on the large changes?
Let us not make large changes, as long as it is working and not making other ongoing changes a hassle. Refactor only when there is a good reason.
_plopp_maskis a special property we have added on mpl Line object and is used to identify the points to consider when trying to autoscale the axes.This was discovered when resizing rectangles in the rectangle selection notebook.
Not sure how to unit-test, as
pickevents currently cannot be simulated.Suggestions welcome!