Repository: ManiacalLabs/BiblioPixel (python)

Issue:
`bp -v ../BiblioPixelAnimations/Projects/circle.json` fails
bp -v ../BiblioPixelAnimations/Projects/circle.json 
WARNING - deprecated - Circle.lastRing
ERROR - failed - Unable to create animation for {'bgcolor': 'black', 'port': 5000, 'title': 'circle Demo', 'external_access': True, 'name_map': {'Swirl': 9, 'ArcRotate': 1, 'PinWheel': 8, 'FireFlies': 5, 'CircleClock': 3, 'ArcClock': 0, 'HyperspaceRainbow': 7, 'OFF_ANIM': 10, 'Diag': 4, 'CircleBloom': 2, 'Hyperspace': 6}, 'animations': [<BiblioPixelAnimations.circle.arc_clock.ArcClock object at 0x106e4c4e0>, <BiblioPixelAnimations.circle.arc_rotate.ArcRotate object at 0x106e4c5c0>, <BiblioPixelAnimations.circle.bloom.CircleBloom object at 0x106e4c6d8>, <BiblioPixelAnimations.circle.circle_clock.CircleClock object at 0x106e4c7f0>, <BiblioPixelAnimations.circle.diag.Diag object at 0x106e4c908>, <BiblioPixelAnimations.circle.fireflies.FireFlies object at 0x106e4ca20>, <BiblioPixelAnimations.circle.hyperspace.Hyperspace object at 0x106e4cb38>, <BiblioPixelAnimations.circle.hyperspace.HyperspaceRainbow object at 0x106b44240>, <BiblioPixelAnimations.circle.pinwheel.PinWheel object at 0x106b44e10>, <BiblioPixelAnimations.circle.swirl.Swirl object at 0x106b44ef0>, <bibliopixel.animation.off.Off object at 0x106b44400>], 'default': None}
ERROR - failed - 
Traceback (most recent call last):
  File "/development/BiblioPixel/bibliopixel/animation/animation.py", line 20, in construct
    a = cls(project.layout, **desc)
  File "/development/BiblioPixel/bibliopixel/animation/remote/control.py", line 114, in __init__
    self.index = self.name_map[DEFAULT_OFF]  # start with default animation
  File "/development/BiblioPixel/bibliopixel/animation/wrapper.py", line 30, in index
    index = self.animations._index(index)
AttributeError: 'list' object has no attribute '_index'


Pull request description:
Compute animation.name in Animation.pre_recursion (fix #722 and #723)

Patch:
diff --git a/bibliopixel/animation/collection.py b/bibliopixel/animation/collection.py
index c7ffe80..aa2daba 100644
--- a/bibliopixel/animation/collection.py
+++ b/bibliopixel/animation/collection.py
@@ -12,6 +12,7 @@ class Collection(animation.Animation):
     @staticmethod
     def pre_recursion(desc):
         animations = []
+        name_count = {}
 
         for a in desc['animations']:
             loaded = load.load_if_filename(a)
@@ -36,6 +37,10 @@ class Collection(animation.Animation):
             run.update(animation.get('run', {}))
             animation['run'] = run
 
+            datatype = animation['datatype']
+            name = animation.setdefault('name', datatype.__name__)
+            name_count[name] = 1 + name_count.get(name, 0)
+
             # Children without fps or sleep_time get it from their parents.
             if not ('fps' in run or 'sleep_time' in run):
                 if 'fps' in desc['run']:
@@ -45,6 +50,14 @@ class Collection(animation.Animation):
 
             animations.append(animation)
 
+        dupes = {k: 1 for k, v in name_count.items() if v > 1}
+        for a in animations:
+            name = a['name']
+            count = dupes.get(name)
+            if count:
+                dupes[name] += 1
+                a['name'] += '_' + str(count - 1)
+
         desc['animations'] = animations
         return desc
 
@@ -52,7 +65,7 @@ class Collection(animation.Animation):
 
     def __init__(self, layout, animations=None, **kwds):
         super().__init__(layout, **kwds)
-        self.animations = animations or []
+        self.animations = _AnimationList(animations or [])
         self.internal_delay = 0  # never wait
 
     # Override to handle all the animations
@@ -70,7 +83,6 @@ class Collection(animation.Animation):
         self.animations.append(anim)
 
     def pre_run(self):
-        self.animations = _AnimationList(self.animations)
         for a in self.animations:
             a.pre_run()
 
@@ -83,6 +95,29 @@ class _AnimationList:
     def _index(self, i):
         return i if isinstance(i, int) else self._names[i]
 
+    def append(self, animation):
+        # We're forced to do the "fill in a name and make sure it's
+        # unique" in two different ways, because of this method.
+        #
+        # TODO: We should disallow APIs from doing surgery on
+        # self.animations after construction.  Right now only
+        # Remote does this, or legacy code that calls add_animation.
+        # That code should go on `cls.pre_recursion( )`.
+        self._animations.append(animation)
+        try:
+            base_name = animation.name
+        except AttributeError:
+            base_name = animation.__class__.__name__
+
+        count = 0
+        name = base_name
+
+        while name in self._names:
+            name = '%s_%d' % (base_name, count)
+            count += 1
+
+        animation.name = name
+
     def __getitem__(self, i):
         return self._animations[self._index(i)]
 
diff --git a/bibliopixel/main/common_flags.py b/bibliopixel/main/common_flags.py
index 26c697d..79f504b 100644
--- a/bibliopixel/main/common_flags.py
+++ b/bibliopixel/main/common_flags.py
@@ -167,7 +167,7 @@ def _make_project_flags(args):
 
 def make_project(args, desc, root_file=None):
     project_flags = _make_project_flags(args)
-    return project.project(project_flags, desc, root_file=root_file)
+    return project.project(desc, project_flags, root_file=root_file)
 
 
 # Help messages.
diff --git a/bibliopixel/project/cleanup.py b/bibliopixel/project/cleanup.py
index c4d547d..bd0102f 100644
--- a/bibliopixel/project/cleanup.py
+++ b/bibliopixel/project/cleanup.py
@@ -42,6 +42,8 @@ def cleanup_animation(desc):
     if not datatype:
         raise ValueError('Missing "datatype" in "animation" section')
 
+    desc['animation'].setdefault('name', datatype.__name__)
+
     from .. animation import sequence
     if not issubclass(datatype, sequence.Sequence):
         # Magic here to allow this to work for non-sequences.
@@ -114,22 +116,6 @@ def cleanup_shape(desc):
     return desc
 
 
-def cleanup_names(animation):
-    animation.name = animation.name or animation.title
-    subs = getattr(animation, 'animations', [])
-    counter = {}
-    for a in subs:
-        cleanup_names(a)
-        counter[a.name] = counter.get(a.name, 0) + 1
-
-    dupes = {k: 1 for k, v in counter.items() if v > 1}
-    for a in subs:
-        count = dupes.get(a.name)
-        if count:
-            dupes[a.name] += 1
-            a.name = '%s_%d' % (a.name, count - 1)
-
-
 def cleanup(desc):
     desc = cleanup_aliases(desc)
     desc = cleanup_animation(desc)
diff --git a/bibliopixel/project/load.py b/bibliopixel/project/load.py
index f9e83d5..7e18181 100644
--- a/bibliopixel/project/load.py
+++ b/bibliopixel/project/load.py
@@ -18,10 +18,11 @@ def data(name, use_json=True):
 
 
 def code(name, python_path=None):
-    return name and loady.code.load_code(name, python_path)
+    return name and loady.code.load_code(name, python_path, recurse=True)
 
 
 def module(name, python_path=None):
+    print('load.module', True)
     return name and loady.code.load_module(name, python_path)
 
 
diff --git a/bibliopixel/project/project.py b/bibliopixel/project/project.py
index f57379f..073455f 100644
--- a/bibliopixel/project/project.py
+++ b/bibliopixel/project/project.py
@@ -46,7 +46,6 @@ class Project:
             self.layout = self.construct_child(**layout)
 
         self.animation = create(animation, 'animation')
-        cleanup.cleanup_names(self.animation)
 
         eq = event_queue.EventQueue(maxsize=event_queue_maxsize)
         self.layout.event_queue = self.animation.event_queue = eq


Test patch:
diff --git a/test/bibliopixel/project/aliases_test.py b/test/bibliopixel/project/aliases_test.py
index 551bb14..c560a01 100644
--- a/test/bibliopixel/project/aliases_test.py
+++ b/test/bibliopixel/project/aliases_test.py
@@ -74,6 +74,4 @@ class AliasTest(unittest.TestCase):
 
 # Aliases that would fail to load at all if they were removed
 FAILED = 'apa102', 'lpd8806', 'pi_ws281x', 'sk9822', 'spi', 'ws2801', 'ws281x'
-
-# Aliases that load and get the wrong value
-NOT_EQUAL = 'reprocess', 'serial'
+NOT_EQUAL = 'serial',
diff --git a/test/bibliopixel/project/cleanup_test.py b/test/bibliopixel/project/cleanup_test.py
index 1c45a6c..d529d10 100644
--- a/test/bibliopixel/project/cleanup_test.py
+++ b/test/bibliopixel/project/cleanup_test.py
@@ -9,6 +9,7 @@ BASE = {
     'animation': {
         'run': {},
         'datatype': OffAnim,
+        'name': 'Off',
         'typename': 'off',
     },
     'controls': [],