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

DynamicMap with streams force callback on first lookup after refresh #858

Merged
merged 11 commits into from Sep 12, 2016
6 changes: 4 additions & 2 deletions holoviews/core/spaces.py
Expand Up @@ -477,6 +477,7 @@ def __init__(self, callback, initial_items=None, **params):

self.call_mode = self._validate_mode()
self.mode = 'bounded' if self.call_mode == 'key' else 'open'
self._dimensionless_cache = False


def _initial_key(self):
Expand Down Expand Up @@ -672,7 +673,8 @@ def __getitem__(self, key):

# Cache lookup
try:
if util.dimensionless_contents(self.streams, self.kdims):
dimensionless = util.dimensionless_contents(self.streams, self.kdims)
if (dimensionless and not self._dimensionless_cache):
raise KeyError('Using dimensionless streams disables DynamicMap cache')
Copy link
Contributor

Choose a reason for hiding this comment

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

From our discussion, we think it ought to be something like:

if (util.dimensionless_contents(self.streams, self.kdims)
                  raise KeyError('Using dimensionless streams disables DynamicMap cache')
elif self._disable_cache_lookup:
                  raise KeyError('Cache lookup disabled')

That way the variable isn't about streams and you can disable cache lookup on dynamic maps not matter how they are (or aren't) using streams.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we decided in the end that it is about streams.

cache = super(DynamicMap,self).__getitem__(key)
# Return selected cache items in a new DynamicMap
Expand Down Expand Up @@ -708,7 +710,7 @@ def _cache(self, key, val):
if self.mode == 'open' and (self.counter % self.cache_interval)!=0:
return
if len(self) >= cache_size:
first_key = next(self.data.iterkeys())
first_key = next(k for k in self.data)
self.data.pop(first_key)
self.data[key] = val

Expand Down
26 changes: 26 additions & 0 deletions holoviews/core/traversal.py
Expand Up @@ -128,3 +128,29 @@ def hierarchical(keys):
store1[v2].append(v1)
hierarchies.append(store2 if hierarchy else {})
return hierarchies


class dimensionless_cache(object):
"""
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.

Given the discussion about, I would call it disable_cache_lookup with constructor:

def __init__(self, obj):

I don't think it needs an enable or disable flag as enable is the default behaviour. Unless you can think of situations where you want to be able to disable the context manager with a boolean? If you can think of such a situation (or just think it makes it more flexible) then

def __init__(self, obj, disable=True):

Would be reasonable. The issue with using either enable or disable as an argument name is that it isn't clear if you are enabling/disabling the cache lookup of enabling/disabling the context manager that is enabling/disabling the cache lookup!

For this reason, maybe something like this would be clearer:

def __init__(self, obj, allow_cache_lookup=False):

Just some things to think about...

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it needs an enable or disable flag as enable is the default behaviour. Unless you can think of situations where you want to be able to disable the context manager with a boolean? If you can think of such a situation (or just think it makes it more flexible) then

The issue is that making context manager conditional is awkward, which is why I moved the boolean onto the object. I agree with renaming the argument and making it optional though.

Context manager which temporarily enables lookup of frame in the
cache on a DynamicMap with dimensionless streams. Allows passing
any Dimensioned object which might contain a DynamicMap and
whether to enable the cache. This allows looking up an item
without triggering the callback. Useful when the object is looked
up multiple times as part of some processing pipeline.
"""

def __init__(self, obj, allow_cache_lookup=True):
self.obj = obj
self._allow_cache_lookup = allow_cache_lookup

def __enter__(self):
self.set_cache_flag(self._allow_cache_lookup)

def __exit__(self, exc_type, exc_val, exc_tb):
self.set_cache_flag(False)

def set_cache_flag(self, value):
self.obj.traverse(lambda x: setattr(x, '_stream_cache_lookup', value),
['DynamicMap'])

2 changes: 1 addition & 1 deletion holoviews/core/util.py
Expand Up @@ -787,7 +787,7 @@ def stream_parameters(streams, no_duplicates=True, exclude=['name']):
If no_duplicates is enabled, a KeyError will be raised if there are
parameter name clashes across the streams.
"""
param_groups = [s.params().keys() for s in streams]
param_groups = [s.contents.keys() for s in streams]
names = [name for group 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 for the fix!


if no_duplicates:
Expand Down
2 changes: 1 addition & 1 deletion holoviews/plotting/mpl/renderer.py
Expand Up @@ -150,7 +150,7 @@ def diff(self, plot):
if self.mode == 'mpld3':
figure_format = 'json'
elif self.fig == 'auto':
figure_format = self.renderer.params('fig').objects[0]
figure_format = self.params('fig').objects[0]
else:
figure_format = self.fig
data = self.html(plot, figure_format, comm=False)
Expand Down
22 changes: 16 additions & 6 deletions holoviews/plotting/plot.py
Expand Up @@ -18,9 +18,10 @@
from ..core.options import Store, Compositor, SkipRendering
from ..core.overlay import NdOverlay
from ..core.spaces import HoloMap, DynamicMap
from ..core.traversal import dimensionless_cache
from ..element import Table
from .util import (get_dynamic_mode, initialize_sampled, dim_axis_label,
attach_streams)
attach_streams, traverse_setter)


class Plot(param.Parameterized):
Expand Down Expand Up @@ -198,6 +199,7 @@ def __init__(self, keys=None, dimensions=None, layout_dimensions=None,
self.ranges = {}
self.renderer = renderer if renderer else Store.renderers[self.backend].instance()
self.comm = None
self._force = True

params = {k: v for k, v in params.items()
if k in self.params()}
Expand Down Expand Up @@ -481,6 +483,7 @@ def refresh(self, **kwargs):
Refreshes the plot by rerendering it and then pushing
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:
Expand Down Expand Up @@ -600,7 +603,9 @@ def _get_frame(self, key):
self.current_key = key
return self.current_frame
elif self.dynamic:
key, frame = util.get_dynamic_item(self.hmap, self.dimensions, key)
with dimensionless_cache(self.hmap, not self._force or not self.drawn):
key, frame = util.get_dynamic_item(self.hmap, self.dimensions, key)
traverse_setter(self, '_force', False)
if not isinstance(key, tuple): key = (key,)
key_map = dict(zip([d.name for d in self.hmap.kdims], key))
key = tuple(key_map.get(d.name, None) for d in self.dimensions)
Expand Down Expand Up @@ -898,17 +903,16 @@ def get_extents(self, overlay, ranges):
class GenericCompositePlot(DimensionedPlot):

def __init__(self, layout, keys=None, dimensions=None, **params):
dynamic, sampled = get_dynamic_mode(layout)
if sampled:
initialize_sampled(layout, dimensions, keys[0])

if 'uniform' not in params:
params['uniform'] = traversal.uniform(layout)

top_level = keys is None
if top_level:
dimensions, keys = traversal.unique_dimkeys(layout)

dynamic, sampled = get_dynamic_mode(layout)
if sampled:
initialize_sampled(layout, dimensions, keys[0])
self.layout = layout
super(GenericCompositePlot, self).__init__(keys=keys,
dynamic=dynamic,
Expand Down Expand Up @@ -944,6 +948,11 @@ def _get_frame(self, key):
dim_keys = zip([d.name for d in self.dimensions
if d in item.dimensions('key')], key)
self.current_key = tuple(k[1] for k in dim_keys)
elif item.traverse(lambda x: x, [DynamicMap]):
with dimensionless_cache(item, not self._force or not self.drawn):
key, frame = util.get_dynamic_item(item, self.dimensions, key)
layout_frame[path] = frame
continue
elif self.uniform:
dim_keys = zip([d.name for d in self.dimensions
if d in item.dimensions('key')], key)
Expand All @@ -957,6 +966,7 @@ def _get_frame(self, key):
layout_frame[path] = obj
else:
layout_frame[path] = item
traverse_setter(self, '_force', False)

self.current_frame = layout_frame
return layout_frame
Expand Down
8 changes: 8 additions & 0 deletions holoviews/plotting/util.py
Expand Up @@ -285,3 +285,11 @@ def append_refresh(dmap):
for stream in dmap.streams:
stream._hidden_subscribers.append(plot.refresh)
return obj.traverse(append_refresh, [DynamicMap])


def traverse_setter(obj, attribute, value):
"""
Traverses the object and sets the supplied attribute on the
object. Supports Dimensioned and DimensionedPlot types.
"""
obj.traverse(lambda x: setattr(x, attribute, value))
19 changes: 17 additions & 2 deletions tests/testplotinstantiation.py
Expand Up @@ -3,17 +3,20 @@
"""

from unittest import SkipTest
from io import BytesIO

import numpy as np
from holoviews import (Dimension, Curve, Scatter, Overlay, DynamicMap,
Store, Image, VLine, NdOverlay, Points)
from holoviews.element.comparison import ComparisonTestCase
from holoviews.streams import PositionXY

# Standardize backend due to random inconsistencies
try:
from matplotlib import pyplot
pyplot.switch_backend('agg')
from holoviews.plotting.mpl import OverlayPlot
from holoviews.plotting.comms import JupyterPushComm
from holoviews.plotting.comms import Comm
mpl_renderer = Store.renderers['matplotlib']
except:
mpl_renderer = None
Expand All @@ -31,7 +34,7 @@ def setUp(self):
if mpl_renderer is None:
raise SkipTest("Matplotlib required to test plot instantiation")
self.default_comm, _ = mpl_renderer.comms['default']
mpl_renderer.comms['default'] = (JupyterPushComm, '')
mpl_renderer.comms['default'] = (Comm, '')

def teardown(self):
mpl_renderer.comms['default'] = (self.default_comm, '')
Expand All @@ -53,6 +56,18 @@ def test_dynamic_nonoverlap(self):
mpl_renderer.get_widget(dmap1 + dmap2, 'selection')


def test_dynamic_streams_refresh(self):
stream = PositionXY()
dmap = DynamicMap(lambda x, y: Points([(x, y)]),
kdims=[], streams=[stream])
plot = mpl_renderer.get_plot(dmap)
plot.initialize_plot()
pre = mpl_renderer(plot, fmt='png')
stream.update(x=1, y=1)
plot.refresh()
post = mpl_renderer(plot, fmt='png')
self.assertNotEqual(pre, post)


class TestBokehPlotInstantiation(ComparisonTestCase):

Expand Down