-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Fixed dynamic stream sources assignment in plotting code #1297
Changes from 15 commits
a283ee7
e60d01c
1fe4859
9248fb9
3ab6adf
9cd0109
ce4c476
75d6261
5941c83
1f78b12
f88b8f4
048642f
521f7be
243bd41
63c50f6
945d363
9779aa6
1be4abe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ def dynamic_mul(*args, **kwargs): | |
element = other[args] | ||
return self * element | ||
callback = Callable(dynamic_mul, inputs=[self, other]) | ||
callback._is_overlay = True | ||
return other.clone(shared_data=False, callback=callback, | ||
streams=[]) | ||
if isinstance(other, UniformNdMapping) and not isinstance(other, CompositeOverlay): | ||
|
@@ -41,7 +42,6 @@ def dynamic_mul(*args, **kwargs): | |
|
||
|
||
|
||
|
||
class CompositeOverlay(ViewableElement, Composable): | ||
""" | ||
CompositeOverlay provides a common baseclass for Overlay classes. | ||
|
@@ -136,7 +136,16 @@ def __add__(self, other): | |
|
||
|
||
def __mul__(self, other): | ||
if not isinstance(other, ViewableElement): | ||
if type(other).__name__ == 'DynamicMap': | ||
from .spaces import Callable | ||
def dynamic_mul(*args, **kwargs): | ||
element = other[args] | ||
return self * element | ||
callback = Callable(dynamic_mul, inputs=[self, other]) | ||
callback._is_overlay = True | ||
return other.clone(shared_data=False, callback=callback, | ||
streams=[]) | ||
elif not isinstance(other, ViewableElement): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We really need to refactor |
||
raise NotImplementedError | ||
return Overlay.from_values([self, other]) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,29 +117,19 @@ def _dynamic_mul(self, dimensions, other, keys): | |
map_obj = self if isinstance(self, DynamicMap) else other | ||
|
||
def dynamic_mul(*key, **kwargs): | ||
key_map = {d.name: k for d, k in zip(dimensions, key)} | ||
layers = [] | ||
try: | ||
if isinstance(self, DynamicMap): | ||
safe_key = () if not self.kdims else key | ||
_, self_el = util.get_dynamic_item(self, dimensions, safe_key) | ||
if self_el is not None: | ||
layers.append(self_el) | ||
else: | ||
layers.append(self[key]) | ||
self_el = self.select(**key_map) if self.kdims else self[()] | ||
except KeyError: | ||
pass | ||
try: | ||
if isinstance(other, DynamicMap): | ||
safe_key = () if not other.kdims else key | ||
_, other_el = util.get_dynamic_item(other, dimensions, safe_key) | ||
if other_el is not None: | ||
layers.append(other_el) | ||
else: | ||
layers.append(other[key]) | ||
other_el = other.select(**key_map) if other.kdims else other[()] | ||
except KeyError: | ||
pass | ||
return Overlay(layers) | ||
callback = Callable(dynamic_mul, inputs=[self, other]) | ||
callback._is_overlay = True | ||
if map_obj: | ||
return map_obj.clone(callback=callback, shared_data=False, | ||
kdims=dimensions, streams=[]) | ||
|
@@ -207,6 +197,7 @@ def dynamic_mul(*args, **kwargs): | |
element = self[args] | ||
return element * other | ||
callback = Callable(dynamic_mul, inputs=[self, other]) | ||
callback._is_overlay = True | ||
return self.clone(shared_data=False, callback=callback, | ||
streams=[]) | ||
items = [(k, v * other) for (k, v) in self.data.items()] | ||
|
@@ -413,7 +404,11 @@ class Callable(param.Parameterized): | |
when composite objects such as Layouts are returned from the | ||
callback. This is required for building interactive, linked | ||
visualizations (for the backends that support them) when returning | ||
Layouts, NdLayouts or GridSpace objects. | ||
Layouts, NdLayouts or GridSpace objects. When chaining multiple | ||
DynamicMaps into a pipeline, the link_inputs parameter declares | ||
whether the visualization generated using this Callable will | ||
inherit the linked streams. This parameter is used as a hint by | ||
the applicable backend. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think this read a bit better:
|
||
|
||
The mapping should map from an appropriate key to a list of | ||
streams associated with the selected object. The appropriate key | ||
|
@@ -429,6 +424,16 @@ class Callable(param.Parameterized): | |
The list of inputs the callable function is wrapping. Used | ||
to allow deep access to streams in chained Callables.""") | ||
|
||
link_inputs = param.Boolean(default=True, doc=""" | ||
If the Callable wraps around other DynamicMaps in its inputs, | ||
determines whether linked streams attached to the inputs are | ||
transferred to the objects returned by the Callable. | ||
|
||
For example the Callable wraps a DynamicMap with an RangeXY | ||
stream, this switch determines whether the corresponding | ||
visualization should update this stream with range changes | ||
originating from the newly generated axes.""") | ||
|
||
memoize = param.Boolean(default=True, doc=""" | ||
Whether the return value of the callable should be memoized | ||
based on the call arguments and any streams attached to the | ||
|
@@ -441,6 +446,8 @@ class Callable(param.Parameterized): | |
def __init__(self, callable, **params): | ||
super(Callable, self).__init__(callable=callable, **params) | ||
self._memoized = {} | ||
self._is_overlay = False | ||
|
||
|
||
@property | ||
def argspec(self): | ||
|
@@ -687,10 +694,7 @@ def clone(self, data=None, shared_data=True, new_type=None, *args, **overrides): | |
# Ensure the clone references this object to ensure | ||
# stream sources are inherited | ||
if clone.callback is self.callback: | ||
clone.callback = self.callback.clone() | ||
if self not in clone.callback.inputs: | ||
with util.disable_constant(clone.callback): | ||
clone.callback.inputs = clone.callback.inputs+[self] | ||
clone.callback = clone.callback.clone(inputs=[self]) | ||
return clone | ||
|
||
|
||
|
@@ -1104,7 +1108,7 @@ def dynamic_hist(obj): | |
adjoin=False, **kwargs) | ||
|
||
from ..util import Dynamic | ||
hist = Dynamic(self, operation=dynamic_hist) | ||
hist = Dynamic(self, link_inputs=False, operation=dynamic_hist) | ||
if adjoin: | ||
return self << hist | ||
else: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -264,6 +264,11 @@ class shade(ElementOperation): | |
and any valid transfer function that accepts data, mask, nbins | ||
arguments.""") | ||
|
||
link_inputs = param.Boolean(default=True, doc=""" | ||
By default, the link_inputs parameter is set to True so that | ||
when applying shade, backends that support linked streams | ||
update RangeXY streams on the inputs of the shade operation.""") | ||
|
||
@classmethod | ||
def concatenate(cls, overlay): | ||
""" | ||
|
@@ -395,6 +400,11 @@ class dynspread(ElementOperation): | |
Higher values give more spreading, up to the max_px | ||
allowed.""") | ||
|
||
link_inputs = param.Boolean(default=True, doc=""" | ||
By default, the link_inputs parameter is set to True so that | ||
when applying dynspread, backends that support linked streams | ||
update RangeXY streams on the inputs of the dynspread operation.""") | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as for the shade operation. |
||
@classmethod | ||
def uint8_to_uint32(cls, img): | ||
shape = img.shape | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,7 @@ | |
from ...element import RGB | ||
from ...streams import Stream, RangeXY, RangeX, RangeY | ||
from ..plot import GenericElementPlot, GenericOverlayPlot | ||
from ..util import dynamic_update, get_sources, attach_streams | ||
from ..util import dynamic_update, attach_streams | ||
from .plot import BokehPlot, TOOLS | ||
from .util import (mpl_to_bokeh, convert_datetime, update_plot, get_tab_title, | ||
bokeh_version, mplcmap_to_palette, py2js_tickformatter, | ||
|
@@ -190,9 +190,18 @@ def _construct_callbacks(self): | |
Initializes any callbacks for streams which have defined | ||
the plotted object as a source. | ||
""" | ||
if isinstance(self, OverlayPlot): | ||
zorders = [] | ||
elif self.batched: | ||
zorders = list(range(self.zorder, self.zorder+len(self.hmap.last))) | ||
else: | ||
zorders = [self.zorder] | ||
|
||
if isinstance(self, OverlayPlot) and not self.batched: | ||
sources = [] | ||
if not self.static or isinstance(self.hmap, DynamicMap): | ||
sources = [(i, o) for i, o in get_sources(self.hmap) | ||
if i in [None, self.zorder]] | ||
sources = [(i, o) for i, inputs in self.stream_sources.items() | ||
for o in inputs if i in zorders] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does being a source have to do with a z-ordering? The latter is a visualization concept so what does that have to with being a source or not? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See below |
||
else: | ||
sources = [(self.zorder, self.hmap.last)] | ||
cb_classes = set() | ||
|
@@ -208,6 +217,7 @@ def _construct_callbacks(self): | |
cbs.append(cb(self, cb_streams, source)) | ||
return cbs | ||
|
||
|
||
def _hover_opts(self, element): | ||
if self.batched: | ||
dims = list(self.hmap.last.kdims) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from __future__ import unicode_literals | ||
from collections import defaultdict | ||
|
||
import numpy as np | ||
import param | ||
|
@@ -7,7 +8,7 @@ | |
Overlay, GridSpace, NdLayout, Store, Dataset) | ||
from ..core.spaces import get_nested_streams, Callable | ||
from ..core.util import (match_spec, is_number, wrap_tuple, basestring, | ||
get_overlay_spec, unique_iterator) | ||
get_overlay_spec, unique_iterator, unique_iterator) | ||
|
||
|
||
def displayable(obj): | ||
|
@@ -72,6 +73,68 @@ def collate(obj): | |
raise Exception(undisplayable_info(obj)) | ||
|
||
|
||
def isoverlay_fn(obj): | ||
""" | ||
Determines whether object is a DynamicMap returning (Nd)Overlay types. | ||
""" | ||
return isinstance(obj, DynamicMap) and (isinstance(obj.last, CompositeOverlay)) | ||
|
||
|
||
def compute_overlayable_zorders(obj, path=[]): | ||
""" | ||
Traverses an overlayable composite container to determine which | ||
objects are associated with specific (Nd)Overlay layers by | ||
z-order, making sure to take DynamicMap Callables into | ||
account. Returns a mapping between the zorders of each layer and a | ||
corresponding lists of objects. | ||
|
||
Used to determine which overlaid subplots should be linked with | ||
Stream callbacks. | ||
""" | ||
path = path+[obj] | ||
zorder_map = defaultdict(list) | ||
|
||
# Process non-dynamic layers | ||
if not isinstance(obj, DynamicMap): | ||
if isinstance(obj, CompositeOverlay): | ||
for z, o in enumerate(obj): | ||
zorder_map[z] = [o, obj] | ||
else: | ||
if obj not in zorder_map[0]: | ||
zorder_map[0].append(obj) | ||
return zorder_map | ||
|
||
isoverlay = isinstance(obj.last, CompositeOverlay) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As the previous conditional returned for non-DynamicMaps this must be a DynamicMap now and there is no guarantee that there is anything in the cache for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The relevant nodes are evaluated by this point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, just to be sure I would assert that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, not all paths have to have been evaluated, that would break everything. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I agree. |
||
isdynoverlay = obj.callback._is_overlay | ||
if obj not in zorder_map[0] and not isoverlay: | ||
zorder_map[0].append(obj) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this would execute if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is in plotting, the graph has to have been evaluated by this point, even if certain (unimportant) intermediate nodes haven't been. |
||
|
||
# Process the inputs of the DynamicMap callback | ||
dmap_inputs = obj.callback.inputs if obj.callback.link_inputs else [] | ||
for z, inp in enumerate(dmap_inputs): | ||
if any(not (isoverlay_fn(p) or p.last is None) for p in path) and isoverlay_fn(inp): | ||
# Skips branches of graph that collapse Overlay layers | ||
# to avoid adding layers that have been reduced or removed | ||
continue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if you have an operation that takes in an overlay of 3 things and returns an overlay of 2 things? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now handled. |
||
|
||
# Recurse into DynamicMap.callback.inputs and update zorder_map | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this would not happen for a DynamicMap callback with inputs that is user defined i.e There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's correct, the branch below handles that by iterating over the items in the user defined DynamicMap, which will ensure the zorder is maintained but won't attempt to figure out linked inputs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. As we discussed, this should be ok as the user can set things up explicitly if they want to transfer sources around (which means they shouldn't ever need to use Seems fine. |
||
z = z if isdynoverlay else 0 | ||
dinputs = compute_overlayable_zorders(inp, path=path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe call this |
||
offset = max(zorder_map.keys()) | ||
for k, v in dinputs.items(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of |
||
zorder_map[offset+k+z] = list(unique_iterator(zorder_map[offset+k+z]+v)) | ||
|
||
# If object branches but does not declare inputs (e.g. user defined | ||
# DynamicMaps returning (Nd)Overlay) add the items on the DynamicMap.last | ||
found = any(isinstance(p, DynamicMap) and p.callback._is_overlay for p in path) | ||
if found and isoverlay and not isdynoverlay: | ||
offset = max(zorder_map.keys()) | ||
for z, o in enumerate(obj.last): | ||
if o not in zorder_map[offset+z]: | ||
zorder_map[offset+z].append(o) | ||
return zorder_map | ||
|
||
|
||
def initialize_dynamic(obj): | ||
""" | ||
Initializes all DynamicMap objects contained by the object | ||
|
@@ -311,31 +374,6 @@ def append_refresh(dmap): | |
return obj.traverse(append_refresh, [DynamicMap]) | ||
|
||
|
||
def get_sources(obj, index=None): | ||
""" | ||
Traverses Callable graph to resolve sources on | ||
DynamicMap objects, returning a list of sources | ||
indexed by the Overlay layer. | ||
""" | ||
layers = [(index, obj)] | ||
if not isinstance(obj, DynamicMap) or not isinstance(obj.callback, Callable): | ||
return layers | ||
index = 0 if index is None else int(index) | ||
for o in obj.callback.inputs: | ||
if isinstance(o, Overlay): | ||
layers.append((None, o)) | ||
for i, o in enumerate(overlay): | ||
layers.append((index+i, o)) | ||
index += len(o) | ||
elif isinstance(o, DynamicMap): | ||
layers += get_sources(o, index) | ||
index = layers[-1][0]+1 | ||
else: | ||
layers.append((index, o)) | ||
index += 1 | ||
return layers | ||
|
||
|
||
def traverse_setter(obj, attribute, value): | ||
""" | ||
Traverses the object and sets the supplied attribute on the | ||
|
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.
Needs a lot more explanation as almost none of the required concepts exist at this level:
As a very rough suggestion, it should be something along the lines of: