Dynamic Callable API #951

philippjfr · 2016-10-24T20:54:10Z

As described in #895, we needed a way to access streams on DynamicMaps wrapping other DynamicMaps or overlays containing one or more DynamicMaps. This PR implements a Callable object, which is used to wrap the DynamicMap callbacks generated by all internal operations. This makes it possible to access streams and stream sources on nested DynamicMaps and hook them up appropriately in the plotting code.

Currently the Callable object is very straightforward, exposing a callable_function and objects parameter, which correspond to the callback and the list of objects the callback wraps around, e.g. in an overlaying operation between a DynamicMap and a Points Elements, the callable_function would be a function which overlays the two objects and the objects a list containing both objects.

Using two helper functions get_streams and get_sources we can then look up all the streams and sources that are associated with a particular plot and more specifically a specific layer in an Overlay.

The PR is still lacking docstrings and tests but it already works.

The best example to confirm it is working is this example of two separate selection callbacks attached to two separate data sources:

%%opts Points [tools=['box_select', 'lasso_select', 'tap']]
points = hv.Points(np.random.multivariate_normal((0, 0), [[1, 0.1], [0.1, 1]], (1000,)))
points2 = hv.Points(np.random.multivariate_normal((0, 0), [[1, 0.1], [0.1, 1]], (1000,)))

sel1 = Selection1D()
sel2 = Selection1D()
hv.DynamicMap(lambda index: points, kdims=[], streams=[sel1]) *\
hv.DynamicMap(lambda index: hv.HLine(points['y'][index].mean() if index else -10), kdims=[], streams=[sel1]) *\
hv.DynamicMap(lambda index: points2, kdims=[], streams=[sel2]) *\
hv.DynamicMap(lambda index: hv.HLine(points2['y'][index].mean() if index else -10), kdims=[], streams=[sel2])

Allows traversing DynamicMap callbacks to get streams and sources

jlstevens · 2016-10-30T21:02:35Z

I've skimmed the PR and now I'm looking at it in detail. Overall it looks fine but I need to think about the implications for the user quite carefully.

My first thought was...could we offer a decorator that users can easily use to wrap any callable? A decorator would look nice and clean and would be very easy for the user to add at any point. In fact, couldn't this transformation be applied dynamically? For instance, the wrapped callback could be an underscore attribute in DynamicMap as soon as it is declared so that the user doesn't even have to worry or know about any of this?

Perhaps you have already done all this this and I'll find out once I look a bit more closely!

jbednar · 2016-10-30T21:10:07Z

holoviews/core/spaces.py

+    that the operation and the objects that are part of the operation
+    are made available. This allows traversing a DynamicMap that wraps
+    multiple operations and is primarily used to extracting any
+    streams attached to the object.


jlstevens · 2016-10-30T21:22:21Z

@philippjfr I would be interested to see the same example you posted above as DynamicMap * DynamicMap. I think that would give me a better insight into how it works as well as a taste of what users might want to do in some situations.

jlstevens · 2016-10-31T00:02:44Z

holoviews/core/spaces.py

+def get_streams(dmap):
+    """
+    Get streams from DynamicMap with Callable callback.
+    """


Might say something like 'Get all (potentially nested) streams ...'

jlstevens · 2016-10-31T00:08:52Z

holoviews/core/overlay.py

-            return Dynamic(other, operation=dynamic_mul)
+            callback = Callable(callable_function=dynamic_mul,
+                                inputs=[self, other])
+            return other.clone(shared_data=False, callback=callback,


Wondering whether it should be self.clone, or other.clone or maybe a new DynamicMap declaration entirely. I see this is in the condition where other is a DynamicMapbut is this definitely right in terms of kdims? I need to think about it more...

Can't this stuff relying on other being DynamicMap be moved to _dynamic_mul? There is already this condition in __mul__:

if isinstance(self, DynamicMap) or isinstance(other, DynamicMap): return self._dynamic_mul(dimensions, other, super_keys)

If all the logic regarding dynamic could move to _dynamic_mul, that would be cleaner...

Wondering whether it should be self.clone, or other.clone or maybe a new DynamicMap declaration entirely. I see this is in the condition where other is a DynamicMapbut is this definitely right in terms of kdims?

Yes, this is the condition where self is a single Element or Overlay.

If all the logic regarding dynamic could move to _dynamic_mul, that would be cleaner...

This is the __mul__ implementation on Overlayable, it doesn't have _dynamic_mul, because I'd like to avoid inline imports.

jlstevens · 2016-10-31T00:13:19Z

holoviews/core/spaces.py

@@ -689,7 +726,8 @@ def __getitem__(self, key):

        # Cache lookup
        try:
-            dimensionless = util.dimensionless_contents(self.streams, self.kdims)
+            dimensionless = util.dimensionless_contents(get_streams(self),
+                                                        self.kdims, False)
            if (dimensionless and not self._dimensionless_cache):


Using no_duplicates=False would be clearer here...

jlstevens · 2016-10-31T00:15:21Z

holoviews/plotting/renderer.py

@@ -204,7 +204,7 @@ def _validate(self, obj, fmt):
            if (((len(plot) == 1 and not plot.dynamic)
                or (len(plot) > 1 and self.holomap is None) or
                (plot.dynamic and len(plot.keys[0]) == 0)) or
-                not unbound_dimensions(plot.streams, plot.dimensions)):
+                not unbound_dimensions(plot.streams, plot.dimensions, False)):


Again, no_duplicates=False would be clearer here...

jlstevens · 2016-10-31T00:16:01Z

holoviews/core/spaces.py

+
+
+def get_streams(dmap):
+    """


Would get_nested_streams or nested_streams be a better name?

jlstevens · 2016-10-31T00:19:29Z

holoviews/plotting/util.py

+    Traverses Callable graph to resolve sources on
+    DynamicMap objects, returning a list of sources
+    indexed by the Overlay layer.
+    """


Can't this be simplified to:

if isinstance(obj, DynamicMap) and isinstance(obj.callback, Callable): return [(index, obj)]

leaving the rest of the function to handle DynamicMaps with Callable callbacks?

Not quite but it could definitely be refactored more nicely.

Ah...given it is:

+ if isinstance(obj, DynamicMap): + if isinstance(obj.callback, Callable): .... + else: + return [(index, obj)] + else: + return [(index, obj)]

I think I meant:

if not isinstance(obj, DynamicMap) or not isinstance(obj.callback, Callable): return [(index, obj)]

jlstevens · 2016-10-31T00:21:02Z

holoviews/plotting/widgets/__init__.py

@@ -108,7 +108,11 @@ def __init__(self, plot, renderer=None, **params):
        super(NdWidget, self).__init__(**params)
        self.id = plot.comm.target if plot.comm else uuid.uuid4().hex
        self.plot = plot
-        self.dimensions, self.keys = drop_streams(plot.streams,


I assume this bit is simply a bug fix and otherwise unrelated?

Before this wasn't an issue, so not unrelated.

Ok...seems to be a result of plot.streams potentially having many more instances due to the nesting than before...

Right in these cases it just needs to know if a kdim has a corresponding stream or not, there is no actual clash at the DynamicMap level because each level of wrapping, i.e. all operations you apply, resolve their own streams.

jlstevens · 2016-10-31T00:24:24Z

holoviews/util.py

+                self.p.kwargs.update(kwargs)
+                _, el = util.get_dynamic_item(map_obj, map_obj.kdims, key)
+                return self._process(el, key)
+        if isinstance(self.p.operation, ElementOperation):


Couldn't self.p.operation be folded into callable_function so you only need Callable and not OperationCallable?

No because Dynamic performs some wrapping around the operation.

I'm a little confused. _dynamic_operation seems to be called once here and I don't see the operation parameter then being inspected/used on the callback variable anywhere in Dynamic. I am yet to spot where the operation parameter of OperationCallable is used...so I'm assuming I've missed it.

It is never used, but is useful information and will in future be used to look up the outputs.

Ah sorry, I'm confused different bit of code.

if isinstance(self.p.operation, ElementOperation): kwargs = {k: v for k, v in self.p.kwargs.items() if k in self.p.operation.params()} return self.p.operation.process_element(element, key, **kwargs)

jlstevens · 2016-10-31T00:31:59Z

holoviews/plotting/bokeh/element.py

-                     if type(stream) in registry and streams}
+            sources = [(self.zorder, self.hmap.last)]
+        cb_classes = set()
+        for _, source in sources:


So the key difference seems to be that you can now have multiple sources whereas before there was only one. And the sources are now retrieved recursively via get_sources which works via the Callable instances...

philippjfr · 2016-10-31T00:53:00Z

Made the changes you suggested.

jlstevens · 2016-10-31T00:59:26Z

Ok, I feel I mostly understand it now... the most magical bit (i.e the bit I couldn't figure out!) seems to be in get_sources but everything else makes sense.

I'll merge now on the understanding that we now have a plan to improve the user API side of things. This PR shows that this approach will allow nested DynamicMaps to work properly with streams.

philippjfr added 5 commits October 30, 2016 19:09

Added Callable and OperationCallable

5558dbe

Allows traversing DynamicMap callbacks to get streams and sources

Process Callable streams on Plots

4a47dff

Register bokeh callbacks for Callable streams

52d716c

Fixed bokeh ElementPlot.current_handles bug

67e5a39

Fixed streams bug in GenericCompositePlot

628c19e

philippjfr force-pushed the dynamic_callable branch from bb87184 to 628c19e Compare October 30, 2016 19:10

philippjfr added 3 commits October 30, 2016 19:12

Updated doc/builder

e3b2b88

Added docstrings for Callable classes

2361036

Small fix for Dynamic

f710fa2

jbednar reviewed Oct 30, 2016

View reviewed changes

Renamed Callable.objects to Callable.inputs

ca945bf

jlstevens reviewed Oct 31, 2016

View reviewed changes

Cleanup of Callable changes

e019eab

jlstevens merged commit d74a10f into master Oct 31, 2016

philippjfr deleted the dynamic_callable branch January 7, 2017 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Callable API #951

Dynamic Callable API #951

philippjfr commented Oct 24, 2016 •

edited

Loading

jlstevens commented Oct 30, 2016

jbednar Oct 30, 2016

jlstevens commented Oct 30, 2016

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016 •

edited

Loading

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016 •

edited

Loading

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016

philippjfr Oct 31, 2016

philippjfr Oct 31, 2016

philippjfr Oct 31, 2016

jlstevens Oct 31, 2016

philippjfr commented Oct 31, 2016

jlstevens commented Oct 31, 2016

Dynamic Callable API #951

Dynamic Callable API #951

Conversation

philippjfr commented Oct 24, 2016 • edited Loading

jlstevens commented Oct 30, 2016

Choose a reason for hiding this comment

jlstevens commented Oct 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlstevens Oct 31, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippjfr Oct 31, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippjfr commented Oct 31, 2016

jlstevens commented Oct 31, 2016

philippjfr commented Oct 24, 2016 •

edited

Loading

jlstevens Oct 31, 2016 •

edited

Loading

philippjfr Oct 31, 2016 •

edited

Loading