Merge pull request #1271 from ioam/layout_path_inheritance

Ensure old paths aren't dropped when combining Layouts

holoviews/core/layout.py

@@ -16,7 +16,7 @@
 from .dimension import Dimension, Dimensioned, ViewableElement
 from .ndmapping import OrderedDict, NdMapping, UniformNdMapping
 from .tree import AttrTree
-from .util import (unique_array, get_path, make_path_unique)
+from .util import (unique_array, get_path, make_path_unique, int_to_roman)
 from . import traversal
 
 
@@ -408,30 +408,33 @@ def _process_items(cls, vals):
             return vals.data
         elif not isinstance(vals, (list, tuple)):
             vals = [vals]
-        paths = cls._initial_paths(vals)
-        path_counter = Counter(paths)
         items = []
         counts = defaultdict(lambda: 1)
-        counts.update({k: 1 for k, v in path_counter.items() if v > 1})
         cls._unpack_paths(vals, items, counts)
+        items = cls._deduplicate_items(items)
         return items
 
 
     @classmethod
-    def _initial_paths(cls, items, paths=None):
+    def _deduplicate_items(cls, items):
         """
-        Recurses the passed items finding paths for each. Useful for
-        determining which paths are not unique and have to be resolved.
+        Iterates over the paths a second time and ensures that partial
+        paths are not overlapping.
         """
-        if paths is None:
-            paths = []
-        for item in items:
-            path, item = item if isinstance(item, tuple) else (None, item)
-            if type(item) is cls:
-                cls._initial_paths(item.items(), paths)
-                continue
-            paths.append(get_path(item))
-        return paths
+        counter = Counter([path[:i] for path, _ in items for i in range(1, len(path)+1)])
+        if sum(counter.values()) == len(counter):
+            return items
+
+        new_items = []
+        counts = defaultdict(lambda: 0)
+        for i, (path, item) in enumerate(items):
+            if counter[path] > 1:
+                path = path + (int_to_roman(counts[path]+1),)
+            elif counts[path]:
+                path = path[:-1] + (int_to_roman(counts[path]+1),)
+            new_items.append((path, item))
+            counts[path] += 1
+        return new_items
 
 
     @classmethod
@@ -447,8 +450,9 @@ def _unpack_paths(cls, objs, items, counts):
             if type(obj) is cls:
                 cls._unpack_paths(obj, items, counts)
                 continue
-            path = get_path(item)
-            new_path = make_path_unique(path, counts)
+            new = path is None or len(path) == 1
+            path = get_path(item) if new else path
+            new_path = make_path_unique(path, counts, new)
             items.append((new_path, obj))
 
 

holoviews/core/util.py

@@ -1185,17 +1185,24 @@ def get_path(item):
     return tuple(capitalize(fn(p)) for (p, fn) in zip(path, sanitizers))
 
 
-def make_path_unique(path, counts):
+def make_path_unique(path, counts, new):
     """
     Given a path, a list of existing paths and counts for each of the
     existing paths.
     """
-    while path in counts:
+    added = False
+    while any(path == c[:i] for c in counts for i in range(1, len(c)+1)):
         count = counts[path]
         counts[path] += 1
+        if (not new and len(path) > 1) or added:
+            path = path[:-1]
+        else:
+            added = True
         path = path + (int_to_roman(count),)
     if len(path) == 1:
         path = path + (int_to_roman(counts.get(path, 1)),)
+    if path not in counts:
+        counts[path] = 1
     return path
 
 

tests/testutils.py

@@ -497,16 +497,30 @@ def test_get_path_from_item_with_custom_group_and_matching_label(self):
 
     def test_make_path_unique_no_clash(self):
         path = ('Element', 'A')
-        new_path = make_path_unique(path, {})
+        new_path = make_path_unique(path, {}, True)
         self.assertEqual(new_path, path)
 
     def test_make_path_unique_clash_without_label(self):
         path = ('Element',)
-        new_path = make_path_unique(path, {path: 1})
+        new_path = make_path_unique(path, {path: 1}, True)
         self.assertEqual(new_path, path+('I',))
 
     def test_make_path_unique_clash_with_label(self):
         path = ('Element', 'A')
-        new_path = make_path_unique(path, {path: 1})
+        new_path = make_path_unique(path, {path: 1}, True)
         self.assertEqual(new_path, path+('I',))
 
+    def test_make_path_unique_no_clash_old(self):
+        path = ('Element', 'A')
+        new_path = make_path_unique(path, {}, False)
+        self.assertEqual(new_path, path)
+
+    def test_make_path_unique_clash_without_label_old(self):
+        path = ('Element',)
+        new_path = make_path_unique(path, {path: 1}, False)
+        self.assertEqual(new_path, path+('I',))
+
+    def test_make_path_unique_clash_with_label_old(self):
+        path = ('Element', 'A')
+        new_path = make_path_unique(path, {path: 1}, False)
+        self.assertEqual(new_path, path[:-1]+('I',))