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

Implement dynamic relabel, redim and select methods #1029

Merged
merged 14 commits into from
Jan 7, 2017
Merged
Show file tree
Hide file tree
Changes from 10 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
1 change: 1 addition & 0 deletions holoviews/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from .interface import * # noqa (API import)
from .operation import ElementOperation, MapOperation, TreeOperation # noqa (API import)
from .element import * # noqa (API import)
from . import util # noqa (API import)

# Surpress warnings generated by NumPy in matplotlib
# Expected to be fixed in next matplotlib release
Expand Down
22 changes: 13 additions & 9 deletions holoviews/core/dimension.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,15 @@ def relabel(self, label=None, group=None, depth=0):
Assign a new label and/or group to an existing LabelledData
object, creating a clone of the object with the new settings.
"""
keywords = [('label',label), ('group',group)]
obj = self.clone(self.data,
**{k:v for k,v in keywords if v is not None})
if (depth > 0) and getattr(obj, '_deep_indexable', False):
for k, v in obj.items():
obj[k] = v.relabel(group=group, label=label, depth=depth-1)
return obj
new_data = self.data
if (depth > 0) and getattr(self, '_deep_indexable', False):
new_data = []
for k, v in self.data.items():
relabelled = v.relabel(group=group, label=label, depth=depth-1)
new_data.append((k, relabelled))
Copy link
Member Author

Choose a reason for hiding this comment

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

Reimplemented this because assigning this way is inefficient for an internal operation.

keywords = [('label', label), ('group', group)]
kwargs = {k: v for k, v in keywords if v is not None}
return self.clone(new_data, **kwargs)


def matches(self, spec):
Expand Down Expand Up @@ -417,6 +419,7 @@ def map(self, map_fn, specs=None, clone=True):
Recursively replaces elements using a map function when the
specification applies.
"""
if specs and not isinstance(specs, list): specs = [specs]
applies = specs is None or any(self.matches(spec) for spec in specs)

if self._deep_indexable:
Expand Down Expand Up @@ -767,8 +770,9 @@ def select(self, selection_specs=None, **kwargs):

# Apply selection to self
if local_kwargs and matches:
ndims = (len(self.dimensions()) if any(d in self.vdims for d in kwargs)
else self.ndims)
ndims = self.ndims
if any(d in self.vdims for d in kwargs):
ndims = len(self.kdims+self.vdims)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this because len(self.dimensions()) includes cdims?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because it can include the deep dimensions as well which should be processed below.

Copy link
Contributor

@jlstevens jlstevens Jan 7, 2017

Choose a reason for hiding this comment

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

Ok. Makes sense...

select = [slice(None) for _ in range(ndims)]
for dim, val in local_kwargs.items():
if dim == 'value':
Expand Down
122 changes: 100 additions & 22 deletions holoviews/core/spaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,6 @@ def relabel(self, label=None, group=None, depth=1):
return super(HoloMap, self).relabel(label=label, group=group, depth=depth)



def hist(self, num_bins=20, bin_range=None, adjoin=True, individually=True, **kwargs):
histmaps = [self.clone(shared_data=False) for _ in
kwargs.get('dimension', range(1))]
Expand Down Expand Up @@ -655,7 +654,7 @@ def reset(self):
return self


def _cross_product(self, tuple_key, cache):
def _cross_product(self, tuple_key, cache, data_slice):
"""
Returns a new DynamicMap if the key (tuple form) expresses a
cross product, otherwise returns None. The cache argument is a
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to explain the new data_slice argument in the docstring.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, docstrings still need updating. I'd love it if there was some way to inherit method docstrings easily, when you override a method.

Expand Down Expand Up @@ -683,23 +682,52 @@ def _cross_product(self, tuple_key, cache):
val = cache[key]
else:
val = self._execute_callback(*key)
if data_slice:
val = self._dataslice(val, data_slice)
data.append((key, val))
return self.clone(data)
product = self.clone(data)

if data_slice:
from ..util import Dynamic
def dynamic_slice(obj):
return obj[data_slice]
return Dynamic(product, operation=dynamic_slice, shared_data=True)
Copy link
Contributor

@jlstevens jlstevens Jan 7, 2017

Choose a reason for hiding this comment

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

I forget...is there a good reason why this can't be a lambda? (replacing dynamic_slice?)

Copy link
Member Author

Choose a reason for hiding this comment

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

No probably could be.

return product


def _slice_bounded(self, tuple_key):
def _slice_bounded(self, tuple_key, data_slice):
"""
Slices bounded DynamicMaps by setting the soft_ranges on key dimensions.
"""
cloned = self.clone(self)
slices = [el for el in tuple_key if isinstance(el, slice)]
if any(el.step for el in slices):
raise Exception("Slices cannot have a step argument "
"in DynamicMap bounded mode ")
elif len(slices) not in [0, len(tuple_key)]:
raise Exception("Slices must be used exclusively or not at all")
elif not slices:
return None

sliced = self.clone(self)
for i, slc in enumerate(tuple_key):
(start, stop) = slc.start, slc.stop
if start is not None and start < cloned.kdims[i].range[0]:
if start is not None and start < sliced.kdims[i].range[0]:
raise Exception("Requested slice below defined dimension range.")
if stop is not None and stop > cloned.kdims[i].range[1]:
if stop is not None and stop > sliced.kdims[i].range[1]:
raise Exception("Requested slice above defined dimension range.")
cloned.kdims[i].soft_range = (start, stop)
return cloned
sliced.kdims[i].soft_range = (start, stop)
if data_slice:
if not isinstance(sliced, DynamicMap):
return self._dataslice(sliced, data_slice)
else:
from ..util import Dynamic
def dynamic_slice(obj):
return obj[data_slice]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can also be replaced with a lambda...

if len(self):
slices = [slice(None) for _ in range(self.ndims)] + list(data_slice)
sliced = super(DynamicMap, sliced).__getitem__(tuple(slices))
return Dynamic(sliced, operation=dynamic_slice, shared_data=True)
return sliced


def __getitem__(self, key):
Expand All @@ -708,22 +736,22 @@ def __getitem__(self, key):
for a previously generated key that is still in the cache
(for one of the 'open' modes)
"""
tuple_key = util.wrap_tuple_streams(key, self.kdims, self.streams)
# Split key dimensions and data slices
if key is Ellipsis:
return self
elif key == ():
map_slice, data_slice = (), ()
else:
map_slice, data_slice = self._split_index(key)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance _split_index could support the empty tuple directly? What happens with an empty tuple currently? It would be nice if this bit could be:

if key is Ellipsis:
    return self
 map_slice, data_slice = self._split_index(key)

Copy link
Member Author

Choose a reason for hiding this comment

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

NdMapping.__getitem__ does this with empty tuples:

elif indexslice == () and not self.kdims:
     return self.data[()]

Won't work here unfortunately, but I guess I could still handle it in _split_index, NdMapping would just never use that path.

tuple_key = util.wrap_tuple_streams(map_slice, self.kdims, self.streams)

# Validation for bounded mode
if self.mode == 'bounded':
# DynamicMap(...)[:] returns a new DynamicMap with the same cache
if key == slice(None, None, None):
return self.clone(self)

slices = [el for el in tuple_key if isinstance(el, slice)]
if any(el.step for el in slices):
raise Exception("Slices cannot have a step argument "
"in DynamicMap bounded mode ")
if len(slices) not in [0, len(tuple_key)]:
raise Exception("Slices must be used exclusively or not at all")
if slices:
return self._slice_bounded(tuple_key)
sliced = None
Copy link
Contributor

@jlstevens jlstevens Jan 7, 2017

Choose a reason for hiding this comment

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

Stray line that can be removed? (sliced=None)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, thanks.

sliced = self._slice_bounded(tuple_key, data_slice)
if sliced is not None:
return sliced

# Cache lookup
try:
Expand All @@ -742,7 +770,7 @@ def __getitem__(self, key):
"available cache in open interval mode.")

# If the key expresses a cross product, compute the elements and return
product = self._cross_product(tuple_key, cache.data if cache else {})
product = self._cross_product(tuple_key, cache.data if cache else {}, data_slice)
if product is not None:
return product

Expand All @@ -752,10 +780,31 @@ def __getitem__(self, key):
if self.call_mode == 'counter':
val = val[1]

if data_slice:
val = self._dataslice(val, data_slice)
self._cache(tuple_key, val)
return val


def select(self, selection_specs=None, **kwargs):
selection = super(DynamicMap, self).select(selection_specs, **kwargs)
def dynamic_select(obj):
Copy link
Contributor

@jlstevens jlstevens Jan 7, 2017

Choose a reason for hiding this comment

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

Would dynamic_select have any use on its own in util? It seems to offer a select that works with both dynamic and non dynamic objects...(or at least as an operation that can be used to select on dynamic objects).

if selection_specs is not None:
matches = any(obj.matches(spec) for spec in selection_specs)
else:
matches = True
if matches:
return obj.select(**kwargs)
return obj

if not isinstance(selection, DynamicMap):
return dynamic_select(selection)
else:
from ..util import Dynamic
return Dynamic(selection, operation=dynamic_select,
shared_data=True)


def _cache(self, key, val):
"""
Request that a key/value pair be considered for caching.
Expand Down Expand Up @@ -795,6 +844,35 @@ def next(self):
return val


def relabel(self, label=None, group=None, depth=1):
"""
Assign a new label and/or group to an existing LabelledData
object, creating a clone of the object with the new settings.
"""
relabelled = super(DynamicMap, self).relabel(label, group, depth)
if depth > 0:
from ..util import Dynamic
def dynamic_relabel(obj):
return obj.relabel(group=group, label=label, depth=depth-1)
return Dynamic(relabelled, shared_data=True, operation=dynamic_relabel)
return relabelled


def redim(self, specs=None, **dimensions):
"""
Replaces existing dimensions in an object with new dimensions
or changing specific attributes of a dimensions. Dimension
mapping should map between the old dimension name and a
dictionary of the new attributes, a completely new dimension
or a new string name.
"""
redimmed = super(DynamicMap, self).redim(specs, **dimensions)
from ..util import Dynamic
def dynamic_redim(obj):
return obj.redim(specs, **dimensions)
return Dynamic(redimmed, shared_data=True, operation=dynamic_redim)


def groupby(self, dimensions=None, container_type=None, group_type=None, **kwargs):
"""
Implements a dynamic version of a groupby, which will
Expand Down
5 changes: 4 additions & 1 deletion holoviews/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,17 @@ class Dynamic(param.ParameterizedFunction):
kwargs = param.Dict(default={}, doc="""
Keyword arguments passed to the function.""")

shared_data = param.Boolean(default=False, doc="""
Whether the cloned DynamicMap will share the same data.""")
Copy link
Contributor

Choose a reason for hiding this comment

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

As .data for a DynamicMap is the callback, it would be good to explain what sharing/not sharing means here. I'm not quite sure what it means to be honest so it might be something we want to document in other places too.

Copy link
Member Author

Choose a reason for hiding this comment

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

.data on a DynamicMap is not the callback but the cache. Will update the docstring to reflect that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, thanks! I suppose you might want to copy the cache in some cases (e.g for relabel).

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, it's probably mostly going to be used internally.


streams = param.List(default=[], doc="""
List of streams to attach to the returned DynamicMap""")

def __call__(self, map_obj, **params):
self.p = param.ParamOverrides(self, params)
callback = self._dynamic_operation(map_obj)
if isinstance(map_obj, DynamicMap):
dmap = map_obj.clone(callback=callback, shared_data=False,
dmap = map_obj.clone(callback=callback, shared_data=self.p.shared_data,
streams=[])
else:
dmap = self._make_dynamic(map_obj, callback)
Expand Down
29 changes: 28 additions & 1 deletion tests/testdynamic.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import numpy as np
from holoviews import Dimension, DynamicMap, Image, HoloMap
from holoviews import Dimension, DynamicMap, Image, HoloMap, Scatter, Curve
from holoviews.util import Dynamic
from holoviews.element.comparison import ComparisonTestCase

Expand All @@ -11,6 +11,33 @@ def sine_array(phase, freq):
return np.sin(phase + (freq*x**2+freq*y**2))


class DynamicMethods(ComparisonTestCase):

def test_deep_relabel_label(self):
fn = lambda i: Image(sine_array(0,i))
dmap = DynamicMap(fn).relabel(label='Test')
self.assertEqual(dmap[0].label, 'Test')

def test_deep_relabel_group(self):
fn = lambda i: Image(sine_array(0,i))
dmap = DynamicMap(fn).relabel(group='Test')
self.assertEqual(dmap[0].group, 'Test')

def test_redim_dimension_name(self):
fn = lambda i: Image(sine_array(0,i))
dmap = DynamicMap(fn).redim(Default='New')
self.assertEqual(dmap.kdims[0].name, 'New')

def test_deep_redim_dimension_name(self):
fn = lambda i: Image(sine_array(0,i))
dmap = DynamicMap(fn).redim(x='X')
self.assertEqual(dmap[0].kdims[0].name, 'X')

def test_deep_redim_dimension_name_with_spec(self):
fn = lambda i: Image(sine_array(0,i))
dmap = DynamicMap(fn).redim(Image, x='X')
self.assertEqual(dmap[0].kdims[0].name, 'X')


class DynamicTestGeneratorOpen(ComparisonTestCase):

Expand Down