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

Widgets ignore dimension streams #860

Merged
merged 5 commits into from
Sep 12, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions holoviews/core/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -803,15 +803,23 @@ def dimensionless_contents(streams, kdims):
with any of the key dimensions.
"""
names = stream_parameters(streams)
kdim_names = [kdim.name for kdim in kdims]
return [name for name in names if name not in kdim_names]
return [name for name in names if name not in kdims]


def streamless_dimensions(streams, kdims):
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we need a better word for streamless_dimensions. These used to be simply 'key dimensions' before streams came along!

In other words, I would like a way to define these things in terms of what they are instead of what they are not. I can't say I have any great suggestions right now 'widget_dimensions', 'requested_dimensions', 'unbound_dimensions'? Got any ideas?

Return a list of dimensions that have not been associated with
any streams.
"""
params = stream_parameters(streams)
return [d for d in kdims if d not in params]


def wrap_tuple_streams(unwrapped, kdims, streams):
"""
Fills in tuple keys with dimensioned stream values as appropriate.
"""
param_groups = [(s.params().keys(), s) for s in streams]
param_groups = [(s.contents.keys(), s) for s in streams]
pairs = [(name,s) for (group, s) in param_groups for name in group]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Made the same mistake twice!

substituted = []
for pos,el in enumerate(wrap_tuple(unwrapped)):
Expand All @@ -824,6 +832,16 @@ def wrap_tuple_streams(unwrapped, kdims, streams):
return tuple(substituted)


def drop_streams(streams, keys, kdims):
"""
Drop any dimensionsed streams from the keys and kdims.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in 'dimensioned' and maybe add a line saying what it returns.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it is a little odd to put the keys (typically a very long list) before the kdims (much shorter). The order that feels (slightly) more natural to me for the signature is (streams, kdims, keys). As there can be a long list of streams, the next best is probably (kdims, streams, keys).

Just a minor point that you may ignore if you wish!

"""
stream_params = stream_parameters(streams)
inds, dims = zip(*[(ind, kdim) for ind, kdim in enumerate(kdims)
if kdim not in stream_params])
return dims, [tuple(key[ind] for ind in inds) for key in keys]


def itervalues(obj):
"Get value iterator from dictionary for Python 2 and 3"
return iter(obj.values()) if sys.version_info.major == 3 else obj.itervalues()
Expand Down
4 changes: 0 additions & 4 deletions holoviews/plotting/bokeh/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ def _plot_figure(self, idx, fig_format='json'):
msg = dict(patch=json_patch, root=self.plot.state._id)
msg = serialize_json(msg)
return msg
else:
self.plot.push()
return "Complete"


class BokehSelectionWidget(BokehWidget, SelectionWidget):
pass
Expand Down
9 changes: 0 additions & 9 deletions holoviews/plotting/mpl/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ def _plot_figure(self, idx):
return self.renderer.html(self.plot, figure_format, comm=False)


def update(self, key):
if self.plot.dynamic == 'bounded' and not isinstance(key, int):
key = tuple(dim.values[k] if dim.values else k
for dim, k in zip(self.mock_obj.kdims, tuple(key)))
self.plot[key]
self.plot.push()
return '' if self.renderer.mode == 'nbagg' else 'Complete'


def get_frames(self):
if self.renderer.mode == 'nbagg':
manager = self.plot.comm.get_figure_manager()
Expand Down
11 changes: 7 additions & 4 deletions holoviews/plotting/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,9 @@ def refresh(self, **kwargs):
the updated data if the plot has an associated Comm.
"""
traverse_setter(self, '_force', True)
if self.current_key:
self.update(self.current_key)
else:
self.update(0)
key = self.current_key if self.current_key else self.keys[0]
stream_key = util.wrap_tuple_streams(key, self.dimensions, self.streams)
self.update(stream_key)
if self.comm is not None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a relief to see how simple this bit turned out to be.

self.push()

Expand Down Expand Up @@ -573,6 +572,7 @@ def __init__(self, element, keys=None, ranges=None, dimensions=None,
**dict(params, **plot_opts))
if top_level:
self.comm = self.init_comm(element)
self.streams = self.hmap.streams if isinstance(self.hmap, DynamicMap) else []

# Update plot and style options for batched plots
if self.batched:
Expand Down Expand Up @@ -920,6 +920,9 @@ def __init__(self, layout, keys=None, dimensions=None, **params):
**params)
if top_level:
self.comm = self.init_comm(layout)
self.streams = [s for streams in layout.traverse(lambda x: x.streams,
[DynamicMap])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrapping is a bit weird. I would put in layout.traverse(lambda x: x.streams, [DynamicMap]) onto the second line and for s in streams onto the third line.

for s in streams]


def _get_frame(self, key):
Expand Down
7 changes: 4 additions & 3 deletions holoviews/plotting/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import param
from ..core.io import Exporter
from ..core.options import Store, StoreOptions, SkipRendering
from ..core.util import find_file, unicode
from ..core.util import find_file, unicode, streamless_dimensions
from .. import Layout, HoloMap, AdjointLayout
from .widgets import NdWidget, ScrubberWidget, SelectionWidget

Expand Down Expand Up @@ -201,9 +201,10 @@ def _validate(self, obj, fmt):
holomap_formats = self.mode_formats['holomap'][self.mode]

if fmt in ['auto', None]:
if ((len(plot) == 1 and not plot.dynamic)
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)):
(plot.dynamic and len(plot.keys[0]) == 0)) or
not streamless_dimensions(plot.streams, plot.dimensions)):
fmt = fig_formats[0] if self.fig=='auto' else self.fig
else:
fmt = holomap_formats[0] if self.holomap=='auto' else self.holomap
Expand Down
20 changes: 14 additions & 6 deletions holoviews/plotting/widgets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
from ...core import OrderedDict, NdMapping
from ...core.options import Store
from ...core.util import (dimension_sanitizer, safe_unicode,
unique_array, unicode, isnumeric)
unique_array, unicode, isnumeric,
wrap_tuple_streams, drop_streams)
from ...core.traversal import hierarchical

def escape_vals(vals, escape_numerics=True):
Expand Down Expand Up @@ -106,9 +107,8 @@ 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 = plot.dimensions
self.keys = plot.keys

dims, keys = drop_streams(plot.streams, plot.keys, plot.dimensions)
self.dimensions, self.keys = dims, keys
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the variables dims and keys don't seem to be used in this method for anything other than setting self.dimensions and self.keys, I would consider using:

self.dimensions, self.keys = drop_streams(plot.streams, 
                                          plot.keys, 
                                          plot.dimensions)

self.json_data = {}
if self.plot.dynamic: self.embed = False
if renderer is None:
Expand Down Expand Up @@ -194,7 +194,9 @@ def _plot_figure(self, idx):


def update(self, key):
return self._plot_figure(key)
self.plot.update(key)
self.plot.push()
return 'Complete'
Copy link
Contributor

@jlstevens jlstevens Sep 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Update Complete' maybe? Is 'Complete' by itself informative enough? I'm assuming this might be something that appears in the console. If it is a message/part of a protocol, it is probably not worth changing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently part of the protocol, which will change as soon as we use comms for everything.




Expand Down Expand Up @@ -370,4 +372,10 @@ def update(self, key):
if self.plot.dynamic:
key = tuple(dim.values[k] if dim.values else k
for dim, k in zip(self.mock_obj.kdims, tuple(key)))
return self._plot_figure(key)
key = [key[self.dimensions.index(kdim)] if kdim in self.dimensions else None
for kdim in self.plot.dimensions]
key = wrap_tuple_streams(tuple(key), self.plot.dimensions,
self.plot.streams)
self.plot.update(key)
self.plot.push()
return 'Complete'