Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactive: only register once the function parameters watchers #818

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

maximlt
Copy link
Member

@maximlt maximlt commented Aug 3, 2022

Alternative implementation (#768) to fix #763

The problem in #763 was two-fold:

  1. the callback used to update the original pipeline object (self._obj, e.g. a DataFrame) was registered every time a new Interactive instance was created. Even a small pipeline creates many instances so that led to many calls being made when one of the parameters controlling the input function changed.
  2. only the last callback registered was actually updating the right self._obj object. Even if all the instances share the same object, setting a new object in the update callback would not lead to that new object being set on all the instances, just on that particular one. This is why in the previous attempts made to fix this issue, which I've also tried, only setting the callbacks on the root instance (depth == 0) didn't work; the Interactive instance being updated isn't the one that drives the display.

To fix 2., I've added a small abstraction layer using a list as the data holder and properties to access/edit the object held, so that the underlying object can be shared across all the Interactive instances created in a pipeline. This means that any change made to _obj by any Interactive instance will be visible to all the other instances.
To fix 1. I've used depth == 0 to only register the update watchers on the root instance, which now works thanks to the fix for 2.

@maximlt maximlt requested a review from hoxbro August 3, 2022 18:33
@philippjfr
Copy link
Member

I can follow this approach better than the other one, it doesn't monkey-patch the obj, the tests are passing and this PR doesn't have merge conflicts to I'm +1 on merging this PR. Updating a shared list is a little bit ugly but I don't know of a better construct in Python and a class to hold the shared reference seems overkill. Feel free to merge.

@maximlt
Copy link
Member Author

maximlt commented Aug 4, 2022

a class to hold the shared reference seems overkill

Indeed I started with a class but it felt overkill, maybe a dataclass would be a good fit but that's not available in Python 3.6 that is still supported by hvPlot. Thanks merging.

@maximlt maximlt merged commit 87d670f into master Aug 4, 2022
@maximlt maximlt deleted the alt_impl_bound_update_obj branch August 4, 2022 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bound input function called many times on widget change
2 participants