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

Fixed dynamic stream sources assignment in plotting code #1297

Merged
merged 18 commits into from
Apr 17, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 24 additions & 3 deletions holoviews/plotting/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ def isoverlay_fn(obj):
return isinstance(obj, DynamicMap) and (isinstance(obj.last, CompositeOverlay))


def overlay_depth(obj):
"""
Computes the depth of a DynamicMap overlay if it can be determined
otherwise return None.
"""
if isinstance(obj, DynamicMap):
if isinstance(obj.last, CompositeOverlay):
return len(obj.last)
elif obj.last is None:
return None
return 1
else:
return 1


def compute_overlayable_zorders(obj, path=[]):
"""
Traverses an overlayable composite container to determine which
Expand Down Expand Up @@ -108,6 +123,7 @@ def compute_overlayable_zorders(obj, path=[]):
isdynoverlay = obj.callback._is_overlay
if obj not in zorder_map[0] and not isoverlay:
zorder_map[0].append(obj)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would execute if .last is None. Maybe this is ok because you are assuming initial_key is called already?

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

depth = overlay_depth(obj)

# Process the inputs of the DynamicMap callback
dmap_inputs = obj.callback.inputs if obj.callback.link_inputs else []
Expand All @@ -116,13 +132,18 @@ def compute_overlayable_zorders(obj, path=[]):
# Skips branches of graph that collapse Overlay layers
# to avoid adding layers that have been reduced or removed
continue
Copy link
Contributor

@jlstevens jlstevens Apr 17, 2017

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

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

Now handled.

elif depth is not None and depth < overlay_depth(inp):
# Skips branch of graph where the number of elements in an
# overlay has been reduced
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean linking from inputs is disabled for this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it can't figure out which parts of the input operation should continue to be linked. A user may be able to supply that information but at that point its easier for them to declare their own Callable and list the inputs they need explicitly.

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 clarifying!

Copy link
Member Author

@philippjfr philippjfr Apr 17, 2017

Choose a reason for hiding this comment

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

For example in the example used for the unit test they could do this:

combined = (area_redim*curve_redim*area2).map(lambda x: x.clone(x.items()[:2]), Overlay)
combined.callback.inputs += [area_redim, curve_redim]

Copy link
Contributor

Choose a reason for hiding this comment

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

That's not bad at and better than trying to complicate things for what I expect is a fairly rare case anyway.


# Recurse into DynamicMap.callback.inputs and update zorder_map
Copy link
Contributor

Choose a reason for hiding this comment

The 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 isdynoverlay is False because _is_overlay wouldn't be set to True by the user? Not saying this is wrong, just making sure this is what we want...

Copy link
Member Author

Choose a reason for hiding this comment

The 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 isdynoverlay is False because _is_overlay wouldn't be set to True by the user?

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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 _is_overlay.

Seems fine.

z = z if isdynoverlay else 0
dinputs = compute_overlayable_zorders(inp, path=path)
deep_zorders = compute_overlayable_zorders(inp, path=path)
offset = max(zorder_map.keys())
for k, v in dinputs.items():
zorder_map[offset+k+z] = list(unique_iterator(zorder_map[offset+k+z]+v))
for dz, objs in deep_zorders.items():
global_z = offset+dz+z
zorder_map[global_z] = list(unique_iterator(zorder_map[global_z]+objs))

# If object branches but does not declare inputs (e.g. user defined
# DynamicMaps returning (Nd)Overlay) add the items on the DynamicMap.last
Expand Down
78 changes: 57 additions & 21 deletions tests/testplotutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from holoviews.core.options import Store
from holoviews.element.comparison import ComparisonTestCase
from holoviews.element import Curve, Area
from holoviews.plotting.util import linked_zorders
from holoviews.plotting.util import compute_overlayable_zorders

try:
from holoviews.plotting.bokeh import util
Expand All @@ -16,41 +16,41 @@

class TestPlotUtils(ComparisonTestCase):

def test_dynamic_linked_zorders_two_mixed_layers(self):
def test_dynamic_compute_overlayable_zorders_two_mixed_layers(self):
area = Area(range(10))
dmap = DynamicMap(lambda: Curve(range(10)), kdims=[])
combined = area*dmap
combined[()]
sources = linked_zorders(combined)
sources = compute_overlayable_zorders(combined)
self.assertEqual(sources[0], [area])
self.assertEqual(sources[1], [dmap])

def test_dynamic_linked_zorders_two_mixed_layers_reverse(self):
def test_dynamic_compute_overlayable_zorders_two_mixed_layers_reverse(self):
area = Area(range(10))
dmap = DynamicMap(lambda: Curve(range(10)), kdims=[])
combined = dmap*area
combined[()]
sources = linked_zorders(combined)
sources = compute_overlayable_zorders(combined)
self.assertEqual(sources[0], [dmap])
self.assertEqual(sources[1], [area])

def test_dynamic_linked_zorders_two_dynamic_layers(self):
def test_dynamic_compute_overlayable_zorders_two_dynamic_layers(self):
area = DynamicMap(lambda: Area(range(10)), kdims=[])
dmap = DynamicMap(lambda: Curve(range(10)), kdims=[])
combined = area*dmap
combined[()]
sources = linked_zorders(combined)
sources = compute_overlayable_zorders(combined)
self.assertEqual(sources[0], [area])
self.assertEqual(sources[1], [dmap])

def test_dynamic_linked_zorders_two_deep_dynamic_layers(self):
def test_dynamic_compute_overlayable_zorders_two_deep_dynamic_layers(self):
area = DynamicMap(lambda: Area(range(10)), kdims=[])
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
area_redim = area.redim(x='x2')
curve_redim = curve.redim(x='x2')
combined = area_redim*curve_redim
combined[()]
sources = linked_zorders(combined)
sources = compute_overlayable_zorders(combined)
self.assertIn(area_redim, sources[0])
self.assertIn(area, sources[0])
self.assertNotIn(curve_redim, sources[0])
Expand All @@ -60,7 +60,7 @@ def test_dynamic_linked_zorders_two_deep_dynamic_layers(self):
self.assertNotIn(area_redim, sources[1])
self.assertNotIn(area, sources[1])

def test_dynamic_linked_zorders_three_deep_dynamic_layers(self):
def test_dynamic_compute_overlayable_zorders_three_deep_dynamic_layers(self):
area = DynamicMap(lambda: Area(range(10)), kdims=[])
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
curve2 = DynamicMap(lambda: Curve(range(10)), kdims=[])
Expand All @@ -70,7 +70,7 @@ def test_dynamic_linked_zorders_three_deep_dynamic_layers(self):
combined = area_redim*curve_redim
combined1 = (combined*curve2_redim)
combined1[()]
sources = linked_zorders(combined1)
sources = compute_overlayable_zorders(combined1)
self.assertIn(area_redim, sources[0])
self.assertIn(area, sources[0])
self.assertNotIn(curve_redim, sources[0])
Expand All @@ -92,7 +92,7 @@ def test_dynamic_linked_zorders_three_deep_dynamic_layers(self):
self.assertNotIn(curve_redim, sources[2])
self.assertNotIn(curve, sources[2])

def test_dynamic_linked_zorders_three_deep_dynamic_layers_cloned(self):
def test_dynamic_compute_overlayable_zorders_three_deep_dynamic_layers_cloned(self):
area = DynamicMap(lambda: Area(range(10)), kdims=[])
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
curve2 = DynamicMap(lambda: Curve(range(10)), kdims=[])
Expand All @@ -102,7 +102,7 @@ def test_dynamic_linked_zorders_three_deep_dynamic_layers_cloned(self):
combined = area_redim*curve_redim
combined1 = (combined*curve2_redim).redim(y='y2')
combined1[()]
sources = linked_zorders(combined1)
sources = compute_overlayable_zorders(combined1)

self.assertIn(area_redim, sources[0])
self.assertIn(area, sources[0])
Expand All @@ -125,15 +125,15 @@ def test_dynamic_linked_zorders_three_deep_dynamic_layers_cloned(self):
self.assertNotIn(curve_redim, sources[2])
self.assertNotIn(curve, sources[2])

def test_dynamic_linked_zorders_mixed_dynamic_and_non_dynamic_overlays_reverse(self):
def test_dynamic_compute_overlayable_zorders_mixed_dynamic_and_non_dynamic_overlays_reverse(self):
area1 = Area(range(10))
area2 = Area(range(10))
overlay = area1 * area2
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
curve_redim = curve.redim(x='x2')
combined = curve_redim*overlay
combined[()]
sources = linked_zorders(combined)
sources = compute_overlayable_zorders(combined)

self.assertIn(curve_redim, sources[0])
self.assertIn(curve, sources[0])
Expand All @@ -149,13 +149,13 @@ def test_dynamic_linked_zorders_mixed_dynamic_and_non_dynamic_overlays_reverse(s
self.assertNotIn(curve_redim, sources[2])
self.assertNotIn(curve, sources[2])

def test_dynamic_linked_zorders_mixed_dynamic_and_non_dynamic_ndoverlays(self):
def test_dynamic_compute_overlayable_zorders_mixed_dynamic_and_non_dynamic_ndoverlays(self):
ndoverlay = NdOverlay({i: Area(range(10+i)) for i in range(2)})
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
curve_redim = curve.redim(x='x2')
combined = ndoverlay*curve_redim
combined[()]
sources = linked_zorders(combined)
sources = compute_overlayable_zorders(combined)

self.assertIn(ndoverlay[0], sources[0])
self.assertIn(ndoverlay, sources[0])
Expand All @@ -171,13 +171,13 @@ def test_dynamic_linked_zorders_mixed_dynamic_and_non_dynamic_ndoverlays(self):
self.assertIn(curve, sources[2])
self.assertNotIn(ndoverlay, sources[2])

def test_dynamic_linked_zorders_mixed_dynamic_and_non_dynamic_ndoverlays_reverse(self):
def test_dynamic_compute_overlayable_zorders_mixed_dynamic_and_non_dynamic_ndoverlays_reverse(self):
ndoverlay = NdOverlay({i: Area(range(10+i)) for i in range(2)})
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
curve_redim = curve.redim(x='x2')
combined = curve_redim*ndoverlay
combined[()]
sources = linked_zorders(combined)
sources = compute_overlayable_zorders(combined)

self.assertIn(curve_redim, sources[0])
self.assertIn(curve, sources[0])
Expand All @@ -193,7 +193,7 @@ def test_dynamic_linked_zorders_mixed_dynamic_and_non_dynamic_ndoverlays_reverse
self.assertNotIn(curve_redim, sources[2])
self.assertNotIn(curve, sources[2])

def test_dynamic_linked_zorders_three_deep_dynamic_layers_reduced(self):
def test_dynamic_compute_overlayable_zorders_three_deep_dynamic_layers_reduced(self):
area = DynamicMap(lambda: Area(range(10)), kdims=[])
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
curve2 = DynamicMap(lambda: Curve(range(10)), kdims=[])
Expand All @@ -203,7 +203,7 @@ def test_dynamic_linked_zorders_three_deep_dynamic_layers_reduced(self):
combined = (area_redim*curve_redim).map(lambda x: x.get(0), Overlay)
combined1 = combined*curve2_redim
combined1[()]
sources = linked_zorders(combined1)
sources = compute_overlayable_zorders(combined1)

self.assertNotIn(curve_redim, sources[0])
self.assertNotIn(curve, sources[0])
Expand All @@ -218,6 +218,42 @@ def test_dynamic_linked_zorders_three_deep_dynamic_layers_reduced(self):
self.assertNotIn(curve, sources[1])


def test_dynamic_compute_overlayable_zorders_three_deep_dynamic_layers_reduced_layers_by_one(self):
area = DynamicMap(lambda: Area(range(10)), kdims=[])
area2 = DynamicMap(lambda: Area(range(10)), kdims=[])
curve = DynamicMap(lambda: Curve(range(10)), kdims=[])
curve2 = DynamicMap(lambda: Curve(range(10)), kdims=[])
area_redim = area.redim(x='x2')
curve_redim = curve.redim(x='x2')
curve2_redim = curve2.redim(x='x3')
combined = (area_redim*curve_redim*area2).map(lambda x: x.clone(x.items()[:2]), Overlay)
combined1 = combined*curve2_redim
combined1[()]
sources = compute_overlayable_zorders(combined1)

self.assertNotIn(curve_redim, sources[0])
self.assertNotIn(curve, sources[0])
self.assertNotIn(curve2_redim, sources[0])
self.assertNotIn(curve2, sources[0])
self.assertNotIn(area, sources[0])
self.assertNotIn(area_redim, sources[0])
self.assertNotIn(area2, sources[0])

self.assertNotIn(area_redim, sources[1])
self.assertNotIn(area, sources[1])
self.assertNotIn(curve2_redim, sources[1])
self.assertNotIn(curve2, sources[1])
self.assertNotIn(area2, sources[0])

self.assertIn(curve2_redim, sources[2])
self.assertIn(curve2, sources[2])
self.assertNotIn(area_redim, sources[2])
self.assertNotIn(area, sources[2])
self.assertNotIn(area2, sources[0])
self.assertNotIn(curve_redim, sources[2])
self.assertNotIn(curve, sources[2])


class TestBokehUtils(ComparisonTestCase):

def setUp(self):
Expand Down