-
-
Notifications
You must be signed in to change notification settings - Fork 403
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
Introduction of Streams API #832
Conversation
I think this PR is ready for review. Of course, this is only a first cut at streams and more work is needed to integrate them with the plotting system etc. What this PR should do is make streams available so that integration work can begin. |
param.constant = False | ||
self.set_param(**kwargs) | ||
for param in self.params().values(): | ||
param.constant = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So parameters can't be set directly, ever? How would this play with paramnb, where the widgets set the parameter values directly. I was hoping I could simply attach Stream.update as a callback on the paramnb widgets and have it just work, but if the params are constant it will never work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is fine: just subclass from Stream
without declaring the parameters as constant to create a parameterized object to use with paramnb.
What I think I can fix is to restore the constant
state to what it was originally. That way, declaring stream parameters them as constant is recommended but not mandatory and they suddenly become constant when update is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit alarmed by the suggestion to subclass from Stream for creating a parameterized object for use with paramnb -- the whole point of paramnb is to be able to display Jupyter-editable parameter widgets for parameters that are defined independently of any GUI or plotting system. The underlying parameterized object sometimes is created just for the GUI, but in many cases is something that doesn't know or need to know anything about anything to do with GUIs or plotting. Maybe in this particular case it is fine, because people using paramnb together with HoloViews streams are already doing things tied to plotting, but it does alarm me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just one way of doing it, you can easily create a callback to attach to the paramnb widgets which copies the parameterized objects parameters over to a stream. All I wanted to ensure here is that you can create a parameterized object that works both as a Stream and as the input to paramnb.widgets.
@@ -454,6 +463,12 @@ class DynamicMap(HoloMap): | |||
""") | |||
|
|||
def __init__(self, callback, initial_items=None, **params): | |||
|
|||
# Set source to self if not already specified | |||
for stream in params.get('streams',[]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should happen after the parameters have been instantiated. I wouldn't recommend it but someone could set a number of default streams directly on DynamicMap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I'll fix this now.
""" | ||
# Union of stream values | ||
items = [stream.value.items() for stream in streams] | ||
union = dict(kv for kvs in items for kv in kvs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when there are clashes? Shouldn't it warn at least?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I was planning to do that and then forgot.
Once issue is that self.warning
seems to be showing up in the console and not the notebook...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow it works for Jim, not sure why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't always work; sometimes I see messages from param on the console. But usually they go to the notebook.
Apart from my three comments it looks good. Doesn't need to be finalized right now anyway. Once we've hooked everything up we can still reconsider some of the API. |
I believe I've address your three comments. Ready for final review/merge. |
# Currently building a simple set of subscribers | ||
groups = [stream.subscribers + stream._hidden_subscribers for stream in streams] | ||
subscribers = set(s for subscribers in groups for s in subscribers) | ||
for subscriber in subscribers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, shouldn't some order be maintained, so that the subscribers defined on an individual stream are at least executed in sequence and the _hidden_subscribers are executed after all others?
Something like:
from .core import util
groups = [sub for stream in streams for sub in stream.subscribers]
hidden = [sub for stream in streams for sub in stream._hidden_subscribers]
for subscriber in util.unique_iterator(groups+hidden):
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that makes sense mainly because we know that _hidden_subscribers
is the sort of thing we want to batch and execute last.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, currently it is not well defined when the plot redraw would happen.
Looks good, tests will pass. Now we just need to hook it all up, exciting! |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This is the first PR of many that will introduce an exciting new feature to HoloViews 1.7, namely interactive streams. Everything is very much work-in-progress at this stage.
Streams will make it easy to enable interactive visualizations with HoloViews by making it possible for HoloViews plots to react to events. These events may be generated on the server-side from Python or on the client-side via JavaScript.
In this PR, the focus is on the former (no JavaScript) and future PRs will enable interaction in the browser (using the Bokeh backend). Note that the design of streams ensures they are backend agnostic as possible (like everything else in HoloViews!).
The key points about streams:
update
method.DynamicMap
to enable interactivity. The stream parameters are passed as keyword arguments to theDynamicMap
callable when update events occur.This PR is currently work-in-progress but here is a quick preview. Note that this example will work with the matplotlib backend after a future PR is merged to update the plotting system:
Right now, a dummy kdim ('Ignored') is declared which won't be needed in future.
Next the following setup is currently needed. In future, this will happen automatically and will be hidden from the user:
Now when you call update on the stream:
The plot responds to this event:
We plan to include the following streams:
AxisRangeX
,AxisRangeY
,AxisRangeXY
,BoxSelection
as well as more specialized streams such asElementSelector
andDataSelector
. We'll also make it as easy as possible for users to define their own custom stream implementations.Currently some of our plans are outlined on the wiki here and here. These pages are likely to go stale quickly as the implementation evolves.
Exciting stuff and obviously a lot more to come!