GeezerVison

old_data shouldn't be in CallbackData, old_data should be managed by the target graphic event time it handles an event. When the target graphic handles an event, it should have a copy of the feature data at the indices that are being modified and reset the feature at those indices, and then set the feature w.r.t. to the new CallbackData.

To elaborate:

1. The target graphic keeps a private dict of (features, orig_vals) that were modified at certain indices from the previous callback, something in the form {"feature": (indices, orig_vals_at_indices)}, you could call it self._previous_modified. And with the new GraphicFeature class from indexable graphic attributes #78 we won't even need a dict for it, we can just add _modified_indices and _orig_vals_at_modified_indices as private attributes to GraphicFeature 😄 .

2. In _set_feature() it first uses the information from above to reset the feature at those indices to their original values

with #78 it might be possible for us to just pass the GraphicFeature instance directly instead of the string name when calling link, for example:

graphic.link(event_type, target=some_graphic, feature=some_graphic.color...

If info for resetting feature is moved to _set_feature() graphic, then we will no longer need method for _reset_feature()?

we should still have a _reset_feature() to separate functionality

ah I see you've done it like this, but it's better for the target graphic to handle old data because we want the "feature resetting at previously modified indices" to happen regardless of the event type. For example, if we've registered different types of callbacks to keyboard events, and different mouse events, we want the same reset to happen regardless of what previous event.type triggered the previous feature data to change.

I agree! Just wasn't sure at the time the best way to implement, but now that graphic features exist it should be a lot easier to store old data!

the way it's written it'll only work with single integer indices, we can chat about it

once we implement this it'll be much easier #76 (comment)

it would become getattr(self, feature)[indices] = new_data

I think that for the _set_feature method in each Graphic or GraphicCollection we can replace Any for the indices to the indices that are appropriate for that graphic type.

an indices_mapper function should be given source and target, something like this:

indices_mapper(source=self, target=target_info.target, event_indices=click_info

And the user can write their function to take in all 3 and do whatever they want.

yes, I assumed that some general form would need to be established