Repository: ManiacalLabs/BiblioPixel
Pull request #342 (2017-08-03 12:57:46)

Better aliases (fix #339)
Voilà, we have:

* got rid of the three sections for aliases.
* allowed the `$` prefix for alias expansion of segments of typenames.
* and there are now user-definable aliases!
* which can be set, listed and delete from `bibliopixel`.
* and which respect `--external` (renamed to `--isolate`)!


Patch:
diff --git a/bibliopixel/animation/collection.py b/bibliopixel/animation/collection.py
index 53c056a..ecdb720 100644
--- a/bibliopixel/animation/collection.py
+++ b/bibliopixel/animation/collection.py
@@ -34,22 +34,25 @@ class Collection(animation.BaseAnimation):
             return None
 
     def _make_animation(self, a, no_fail=False):
-        a = copy.deepcopy(a)
-
         if isinstance(a, str):
-            desc = {'animation': a}
+            animation = a
+            run = None
+
         elif isinstance(a, dict):
-            if 'animation' in a:
-                # TODO: this hackiness occurs because this indirectly gets
-                # called from different places.  Figure out a better way.
-                desc = a
+            animation = a.get('animation')
+            if animation:
+                # Looks like {'animation': ..., 'run': }
+                run = a.get('run')
             else:
-                desc = {'animation': a}
+                # It's an animation itself.
+                animation, run = a, None
+
         else:
-            desc = {'animation': a[0], 'run': a[1]}
-        desc = aliases.resolve(desc)
+            animation, run = a
+
+        assert animation, '%s:%s:%s' % (a, animation, run)
         try:
-            return project.make_animation(layout=self.layout, **desc)
+            return project.make_animation(self.layout, animation, run)
         except:
             log.error(traceback.format_exc())
             return None
diff --git a/bibliopixel/layout/multimap.py b/bibliopixel/layout/multimap.py
index 0e89d21..1c0abc4 100644
--- a/bibliopixel/layout/multimap.py
+++ b/bibliopixel/layout/multimap.py
@@ -1,4 +1,5 @@
 from . geometry import gen_matrix
+from .. project import aliases
 
 
 class MultiMapBuilder:
@@ -10,7 +11,7 @@ class MultiMapBuilder:
 
     def make_drivers(self, driver, drivers):
         if not drivers:
-            return [self.make_object(**driver)]
+            return [self.make_object(**aliases.resolve(driver))]
 
         if driver:
             # driver is a default for each driver.
@@ -19,6 +20,7 @@ class MultiMapBuilder:
         return [self._make_driver(**d) for d in drivers]
 
     def _make_driver(self, width, height, matrix=None, **kwds):
+        kwds = aliases.resolve(kwds)
         row = gen_matrix(width, height, **(matrix or {}))
         for y, row_entry in enumerate(row):
             self.map.append([i + self.offset for i in row_entry])
diff --git a/bibliopixel/main/alias.py b/bibliopixel/main/alias.py
new file mode 100644
index 0000000..6336447
--- /dev/null
+++ b/bibliopixel/main/alias.py
@@ -0,0 +1,90 @@
+"""
+Converts its arguments between color names and color tuples.
+"""
+
+
+from .. project import aliases, alias_lists
+
+DELETE_ALL_PROMPT = 'Delete all aliases - are you sure? (yN) '
+
+
+def _assign(parts):
+    def assign(i):
+        name, equals, value = parts[i:i + 3]
+        if equals != '=' or not name.isidentifier():
+            raise ValueError()
+
+        return name, value
+
+    # Compute all the assignments first to fail early.
+    assignments = [assign(i) for i in range(0, len(parts), 3)]
+    for name, value in assignments:
+        alias_lists.set_alias(name, value)
+        print('Set:', name, '=', value)
+
+
+def run(args):
+    if args.delete_all:
+        if args.aliases:
+            raise ValueError('--delete-all takes no arguments')
+
+        if not input(DELETE_ALL_PROMPT).lower().startswith('y'):
+            raise ValueError('--delete-all aborted')
+
+        alias_lists.delete_all_alias()
+        print('All aliases deleted.')
+        return
+
+    if not args.aliases:
+        alias_lists.print_aliases(args.builtin, args.by_value)
+        return
+
+    parts = ' = '.join(' '.join(args.aliases).split('=')).split()
+
+    if '=' in parts:
+        if args.delete:
+            raise ValueError('Cannot --delete %s' % ' '.join(args.aliases))
+        try:
+            _assign(parts)
+        except Exception as e:
+            raise ValueError('Bad assignment %s' % ' '.join(args.aliases))
+
+    elif args.delete:
+        success = []
+        for alias in parts:
+            try:
+                alias_lists.delete_alias(alias)
+                success.append(alias)
+            except:
+                print('Alias', alias, 'did not exist')
+        success and print('Deleted aliases:', *success)
+
+    else:
+        for alias in parts:
+            value = alias_lists.get_alias(alias, aliases.ISOLATE)
+            alias_lists.print_alias(alias, value)
+
+
+def set_parser(parser):
+    parser.set_defaults(run=run)
+
+    parser.add_argument(
+        'aliases', nargs='*',
+        help='Names of aliases',
+        default='')
+
+    parser.add_argument(
+        '--builtin', action='store_true',
+        help='List the builtin aliases')
+
+    parser.add_argument(
+        '--by-value', action='store_true',
+        help='Sort aliases by value, not name')
+
+    parser.add_argument(
+        '--delete', action='store_true',
+        help='Delete one or more built-in aliases')
+
+    parser.add_argument(
+        '--delete-all', action='store_true',
+        help='Delete all built-in aliases')
diff --git a/bibliopixel/main/common_flags.py b/bibliopixel/main/common_flags.py
index fb1b614..021add1 100644
--- a/bibliopixel/main/common_flags.py
+++ b/bibliopixel/main/common_flags.py
@@ -41,10 +41,13 @@ Print the current version number of BiblioPixel (%s).
 """ % VERSION
 
 PRESET_HELP = """Filenames for preset library"""
-EXTERNAL_HELP = """\
-Run BiblioPixel as if it were running on an external machine, which means
-that it will not see any local Python classes (because `.` is no longer in
-`sys.path`.
+ISOLATE_HELP = """\
+Run BiblioPixel in isolated mode, where it cannot see your local files.
+This means that it will not see any local Python classes in your directories
+and it won't see your local aliases.
+
+Running your project in isolated mode help makes sure that your project will
+work on other machines without modification.
 """
 
 
@@ -55,6 +58,9 @@ def add_common_flags(parser):
         '--verbose', '-v', action='store_true', help=VERBOSE_HELP)
     parser.add_argument(
         '--version', action='store_true', help=VERSION_HELP)
+    parser.add_argument(
+        '-x', '--isolate', action='store_true', help=ISOLATE_HELP)
+
     if ENABLE_PRESETS:
         parser.add_argument(
             '--presets', help=PRESET_HELP, default=PRESET_LIBRARY_DEFAULT)
@@ -90,9 +96,6 @@ def add_project_flags(parser):
         '-t', '--ledtype', default=None,
         help='Default LED type if no LED type is specified')
 
-    parser.add_argument(
-        '-x', '--external', action='store_true', help=EXTERNAL_HELP)
-
 
 def make_animation(args, desc):
     def get_value(name):
@@ -112,14 +115,14 @@ def make_animation(args, desc):
     if args.numpy:
         project_flags['maker'] = {'use_numpy': True}
 
-    return project.project_to_animation(desc, project_flags, args.external)
+    return project.project_to_animation(desc, project_flags)
 
 
 def extend_path(args):
-    if args.external:
+    if args.isolate:
         path = args.path
     else:
         path = os.getcwd()
         if args.path:
             path += ':' + args.path
-    project.extend_path(path, args.external)
+    project.extend_path(path)
diff --git a/bibliopixel/main/main.py b/bibliopixel/main/main.py
index 1c2d911..a1309d8 100644
--- a/bibliopixel/main/main.py
+++ b/bibliopixel/main/main.py
@@ -2,10 +2,13 @@ import argparse, os, sys
 from . import common_flags
 from .. util import log
 from .. project.importer import import_symbol
+from .. project import aliases
+
 
 __all__ = ['main']
 COMMANDS = (
-    'all_pixel', 'clear_cache', 'color', 'devices', 'demo', 'run', 'update')
+    'alias', 'all_pixel', 'clear_cache', 'color', 'devices', 'demo', 'run',
+    'update')
 MODULES = {c: import_symbol('.' + c, 'bibliopixel.main') for c in COMMANDS}
 
 
@@ -42,11 +45,17 @@ def get_args(argv=sys.argv):
 
 def main():
     args = get_args()
+    run = getattr(args, 'run', None)
+    if not run:
+        print('ERROR: No command entered')
+        print('Valid:', ', '.join(COMMANDS))
+        sys.exit(-1)
+
+    aliases.ISOLATE = args.isolate
     log.set_log_level(args.loglevel)
 
     try:
-        run = getattr(args, 'run', no_command)
-        result = run(args) or 0
+        return run(args) or 0
     except Exception as e:
         if args.verbose:
             raise
diff --git a/bibliopixel/project/alias_lists.py b/bibliopixel/project/alias_lists.py
new file mode 100644
index 0000000..c595d56
--- /dev/null
+++ b/bibliopixel/project/alias_lists.py
@@ -0,0 +1,74 @@
+import operator, os
+from .. util import datafile
+
+USER_ALIAS_FILE = os.path.expanduser('~/.bibliopixel_aliases')
+USER_ALIASES = datafile.DataFile(USER_ALIAS_FILE)
+
+
+BUILTIN_ALIASES = {
+    # drivers
+    'apa102': 'bibliopixel.drivers.API.APA102.APA102',
+    'sk9822': 'bibliopixel.drivers.API.APA102.APA102',
+    'dummy': 'bibliopixel.drivers.dummy_driver.Dummy',
+    'hue': 'bibliopixel.drivers.hue.Hue',
+    'image': 'bibliopixel.drivers.image_sequence.ImageSequence',
+    'lpd8806': 'bibliopixel.drivers.API.LPD8806.LPD8806',
+    'network': 'bibliopixel.drivers.network.Network',
+    'network_udp': 'bibliopixel.drivers.network.NetworkUDP',
+    'serial': 'bibliopixel.drivers.serial.Serial',
+    'simpixel': 'bibliopixel.drivers.SimPixel.SimPixel',
+    'ws281x': 'bibliopixel.drivers.API.WS281X.WS281X',
+    'ws2801': 'bibliopixel.drivers.API.WS2801.WS2801',
+    'spi': 'bibliopixel.drivers.SPI.SPI',
+    'pi_ws281x': 'bibliopixel.drivers.PiWS281X.PiWS281X',
+
+    # layouts
+    'circle': 'bibliopixel.layout.circle.Circle',
+    'cube': 'bibliopixel.layout.cube.Cube',
+    'matrix': 'bibliopixel.layout.matrix.Matrix',
+    'pov': 'bibliopixel.layout.pov.POV',
+    'strip': 'bibliopixel.layout.strip.Strip',
+
+    # animations
+    'off': 'bibliopixel.animation.off.OffAnim',
+    'remote': 'bibliopixel.remote.control.RemoteControl',
+    'matrix_calibration':
+    'bibliopixel.animation.tests.MatrixCalibrationTest',
+    'matrix_test': 'bibliopixel.animation.tests.MatrixChannelTest',
+    'receiver': 'bibliopixel.animation.receiver.BaseReceiver',
+    'sequence': 'bibliopixel.animation.Sequence',
+    'strip_test': 'bibliopixel.animation.tests.StripChannelTest',
+}
+
+
+def get_alias(alias, isolate=False):
+    return (not isolate and USER_ALIASES.get(alias) or
+            BUILTIN_ALIASES.get(alias))
+
+
+def print_alias(alias, value, print=print):
+    if value:
+        print('%s = %s' % (alias, value))
+    else:
+        print('# %s is not defined.' % alias)
+
+
+def print_aliases(builtin, by_value=False, print=print):
+    """
+    Args:
+        by_value: sort either by alias name, or by alias value
+    """
+
+    aliases = BUILTIN_ALIASES if builtin else USER_ALIASES.data
+    if not aliases:
+        print('(no aliases)')
+        return
+
+    key_func = operator.itemgetter(int(by_value))
+    for alias, value in sorted(aliases.items(), key=key_func):
+        print_alias(alias, value, print)
+
+
+set_alias = USER_ALIASES.set
+delete_alias = USER_ALIASES.delete
+delete_all_alias = USER_ALIASES.delete_all
diff --git a/bibliopixel/project/aliases.py b/bibliopixel/project/aliases.py
index 3644eb8..560ba2e 100644
--- a/bibliopixel/project/aliases.py
+++ b/bibliopixel/project/aliases.py
@@ -1,66 +1,45 @@
-import copy
-from .importer import import_symbol
+import re
+from .. util import log
+from . importer import import_symbol
+from . import alias_lists
 
-ALIASES = {
-    'driver': {
-        'apa102': 'bibliopixel.drivers.API.APA102.APA102',
-        'sk9822': 'bibliopixel.drivers.API.APA102.APA102',
-        'dummy': 'bibliopixel.drivers.dummy_driver.Dummy',
-        'hue': 'bibliopixel.drivers.hue.Hue',
-        'image': 'bibliopixel.drivers.image_sequence.ImageSequence',
-        'lpd8806': 'bibliopixel.drivers.API.LPD8806.LPD8806',
-        'network': 'bibliopixel.drivers.network.Network',
-        'network_udp': 'bibliopixel.drivers.network.NetworkUDP',
-        'serial': 'bibliopixel.drivers.serial.Serial',
-        'simpixel': 'bibliopixel.drivers.SimPixel.SimPixel',
-        'ws281x': 'bibliopixel.drivers.API.WS281X.WS281X',
-        'ws2801': 'bibliopixel.drivers.API.WS2801.WS2801',
-        'spi': 'bibliopixel.drivers.SPI.SPI',
-        'pi_ws281x': 'bibliopixel.drivers.PiWS281X.PiWS281X'
-    },
+ALIAS_MARKER = '$'
+SEPARATORS = re.compile(r'[./#]|[^./#]+')
+DEPRECATE_OLD_ALIASES = False
 
-    'layout': {
-        'circle': 'bibliopixel.layout.circle.Circle',
-        'cube': 'bibliopixel.layout.cube.Cube',
-        'matrix': 'bibliopixel.layout.matrix.Matrix',
-        'pov': 'bibliopixel.layout.pov.POV',
-        'strip': 'bibliopixel.layout.strip.Strip',
-    },
+# This can set from the commamnd line with --isolate
+ISOLATE = False
 
-    'animation': {
-        'off': 'bibliopixel.animation.off.OffAnim',
-        'remote': 'bibliopixel.remote.control.RemoteControl',
-        'matrix_calibration':
-        'bibliopixel.animation.tests.MatrixCalibrationTest',
-        'matrix_test': 'bibliopixel.animation.tests.MatrixChannelTest',
-        'receiver': 'bibliopixel.animation.receiver.BaseReceiver',
-        'sequence': 'bibliopixel.animation.Sequence',
-        'strip_test': 'bibliopixel.animation.tests.StripChannelTest',
-    },
-}
 
+def resolve_one(part):
+    if not part.startswith(ALIAS_MARKER):
+        return part
 
-def resolve(*dicts):
-    """Resolve aliases and merge.  Evaluation proceeds from left to right."""
-    def resolver(key, value):
-        if isinstance(value, str):
-            value = {'typename': value}
-        else:
-            value = copy.deepcopy(value)
+    p = alias_lists.get_alias(part[1:], ISOLATE)
+    if p:
+        return p
 
+    raise ValueError('Do not understand alias %s' % part)
+
+
+def resolve(value):
+    if isinstance(value, str):
+        typename, value = value, {}
+    else:
         typename = value.get('typename')
-        if typename and key in ALIASES:
-            value['typename'] = ALIASES[key].get(typename.lower(), typename)
 
-        return value
+    if not typename:
+        raise ValueError('"typename" is not present or empty')
+
+    alias = alias_lists.get_alias(typename, ISOLATE)
 
-    result = {}
+    if alias:
+        if DEPRECATE_OLD_ALIASES:
+            log.warning('Aliases that do not start with $ are deprecated')
 
-    for d in dicts:
-        for key, value in d.items():
-            if key == 'drivers':
-                result['drivers'] = [resolver('driver', d) for d in value]
-            else:
-                result.setdefault(key, {}).update(**resolver(key, value))
+    else:
+        parts = SEPARATORS.findall(typename)
+        alias = ''.join(resolve_one(p) for p in parts)
 
-    return result
+    value['typename'] = alias
+    return value
diff --git a/bibliopixel/project/project.py b/bibliopixel/project/project.py
index 770f610..ee1030f 100644
--- a/bibliopixel/project/project.py
+++ b/bibliopixel/project/project.py
@@ -8,11 +8,12 @@ from .. util import log
 RESERVED_PROPERTIES = 'name', 'data'
 
 ISNT_GIT_PATH_ERROR = """\
-Because the --external flag is set, all paths most start with //git so that
-they are external.  Your path was %s."""
+Because the --isolate flag is set, all paths must start with //git.
+Your path was %s."""
 
 
 def make_animation(layout, animation, run=None):
+    animation = aliases.resolve(animation)
     reserved = {p: animation.pop(p, None) for p in RESERVED_PROPERTIES}
     animation = importer.make_object(layout, **animation)
 
@@ -24,26 +25,30 @@ def make_animation(layout, animation, run=None):
     return animation
 
 
-def extend_path(path, external=False):
+def extend_path(path):
     if not path:
         return
 
-    if external and not all(x.startswith('//git/') for x in path.split(':')):
-        raise ValueError(ISNT_GIT_PATH_ERROR % path)
+    if aliases.ISOLATE:
+        if not all(x.startswith('//git/') for x in path.split(':')):
+            raise ValueError(ISNT_GIT_PATH_ERROR % path)
 
     gitty.sys_path.extend(path)
 
 
-def project_to_animation(desc, default, external=False):
-    project = aliases.resolve(default or {}, desc)
+def project_to_animation(desc, default):
+    project = copy.deepcopy(desc)
 
-    animation = project.pop('animation', None)
-    driver = project.pop('driver', None)
-    drivers = project.pop('drivers', None)
-    layout = project.pop('layout', None)
-    maker = project.pop('maker', None)
-    path = project.pop('path', None)
-    run = project.pop('run', None)
+    def get(name):
+        return project.pop(name, default.get(name))
+
+    animation = get('animation')
+    driver = get('driver')
+    drivers = get('drivers')
+    layout = get('layout')
+    maker = get('maker')
+    path = get('path')
+    run = get('run')
 
     if project:
         log.error('Did not understand sections %s', project)
@@ -57,12 +62,15 @@ def project_to_animation(desc, default, external=False):
     if not (driver or drivers):
         raise ValueError('Projects has neither driver nor drivers sections')
 
-    extend_path(path, external)
+    extend_path(path)
     maker = data_maker.Maker(**(maker or {}))
     make_object = functools.partial(importer.make_object, maker=maker)
 
     builder = MultiMapBuilder(make_object)
     driver_objects = builder.make_drivers(driver, drivers)
+
+    layout = aliases.resolve(layout)
     coord_map = layout.pop('coord_map', builder.map or None)
     layout_object = make_object(driver_objects, coord_map=coord_map, **layout)
+
     return make_animation(layout_object, animation, run)
diff --git a/bibliopixel/util/datafile.py b/bibliopixel/util/datafile.py
index 20028c3..e396b83 100644
--- a/bibliopixel/util/datafile.py
+++ b/bibliopixel/util/datafile.py
@@ -1,4 +1,4 @@
-import json
+import copy, json
 from . import files
 
 
@@ -7,7 +7,6 @@ class DataFile(object):
         self.filename = filename
 
         self.open = open
-        self.data = {}
         self.read()
 
     def read(self):
@@ -20,3 +19,18 @@ class DataFile(object):
 
     def write(self):
         json.dump(self.data, self.open(self.filename, 'w'))
+
+    def get(self, key):
+        return self.data.get(key)
+
+    def set(self, key, value):
+        self.data[key] = value
+        self.write()
+
+    def delete(self, key):
+        del self.data[key]
+        self.write()
+
+    def delete_all(self):
+        self.data.clear()
+        self.write()


Test patch:
diff --git a/test/bibliopixel/project/aliases_test.py b/test/bibliopixel/project/aliases_test.py
index 6d49593..349f046 100644
--- a/test/bibliopixel/project/aliases_test.py
+++ b/test/bibliopixel/project/aliases_test.py
@@ -4,14 +4,18 @@ from bibliopixel.project import aliases
 
 class AliasTest(unittest.TestCase):
     def test_empty(self):
-        self.assertEqual(aliases.resolve({}), {})
+        with self.assertRaises(ValueError):
+            aliases.resolve({})
 
     def test_simple(self):
-        self.assertEqual(aliases.resolve({'layout': {}}), {'layout': {}})
-        d = {'layout': {'typename': 'bibliopixel.layout.circle.Circle'}}
+        d = {'typename': 'bibliopixel.layout.circle.Circle'}
         self.assertEqual(aliases.resolve(d), d)
 
     def test_resolve(self):
-        d = {'layout': {'typename': 'bibliopixel.layout.circle.Circle'}}
-        self.assertEqual(aliases.resolve({'layout': 'circle'}), d)
-        self.assertEqual(aliases.resolve({'layout': {'typename': 'circle'}}), d)
+        d = {'typename': 'bibliopixel.layout.circle.Circle'}
+        self.assertEqual(aliases.resolve('circle'), d)
+        self.assertEqual(aliases.resolve({'typename': 'circle'}), d)
+
+        d['foo'] = 'bar'
+        self.assertEqual(
+            aliases.resolve({'typename': 'circle', 'foo': 'bar'}), d)