Repository: ManiacalLabs/BiblioPixel (python)

Issue:
Get rid of most or all of the global aliases
Need to write this out to figure out how to write a test for it.  You don't have to read it.  :-D

---

_Project-level_ aliases are fantastic - just so useful in keeping projects readable.  Proven value!  Not on the cutting block.

---

However, we have three other features that overlap now, and I think we can lose one - that one being [global aliases](https://github.com/ManiacalLabs/BiblioPixel/blob/master/bibliopixel/project/alias_lists.py#L40) - and still be _perfectly backwards compatible_ to this instant.

Global aliases need additional documentation to the user, are inelegant, arbitrary and have been subject to bugs in the past, so I'd love to just lose them.

---

Let's look at a specific example: the alias [`sequence`](https://github.com/ManiacalLabs/BiblioPixel/blob/master/bibliopixel/project/alias_lists.py#L40).

How can this possibly work??  How can I keep some project that uses `"sequence"` today still working in the future without this line?

Two other features work nicely together.

`python_path` is one - "where do we look to resolve types?".  (Actually, perhaps this is a bad name, because it's not a file path, but something like `bibliopixel.animation` or `bibliopixel.drivers`.)

Users don't need to know about this (though an ambitious user could override it in a project) but there's a path that's passed to classes at construction saying, "Where do we look for contained types?" - and if it's not overridden, it's `bibliopixel.animation.` for animations, `bibliopixel.drivers` for drivers, etc.

The other is "smart resolution" - we handle "files containing a class of approximately the same name" very well (or "files with a single name of the correct type", etc., we're very friendly in opening Python files). So if there's a file my_anim.py, it'll recognized an animation in it that look like `my_anim`, `myanim`, `MyAnim` (as long as there is only one).  It's not much code and it makes tons of cruft just vanish.

---

So suppose I delete the `"sequence"` alias but use it anyway.  What happens?  

Well, after failing to resolve this in the local directory, it tries to resolve this symbol using the python_path `bibliopixel.animation`.  

It resolves to `bibliopixel.animation.sequence` which is already a module, and therefore a namespace, and then uses smart resolution to come up with `bibliopixel.animation.sequence` - same as is already there!

So my theory is I could delete a lot of these outright, and most of the others I could replace with importing symbols from one namespace into another, worst case in __init__.py.

--

The test! 

* For each existing alias:
  * deduce the section (`bibliopixel.animation` etc)
  * run a call to `construct.to_type_constructor` with the alias 
  * `construct.to_type_constructor` is post-alias but uses the python_path from smart resolution
  * check to see that we're getting the same result 
  * ?
  * Profit!

That wasn't so hard once I thought it out.








Pull request:
Remove most global aliases (fix #632)
This was not entirely a success because I couldn't remove all of them, but it's a good start.  I want to avoid documenting them. :-D

I first write a test that identifies aliases that are redundant due to smart resolution, but leaves them alone.

Then there's code to make more aliases redundant, often by renaming the file to be what I'm now calling "standard form" - where the filename and the classname it exports are "the same" (ignoring case and underscores and the trailing .py) so we get e.g `.devices.dummy.Dummy` and `.animations.off.Off`.

I left variables there to make it all backwards compatible of course.

I found some interesting secrets on the way, such as the fact that the symbol `bibliopixel.layout.matrix.matrix` is in fact... `bibliopixel.layout.matrix_drawing`!  (That fixes this, which isn't 100% backwards compatible, but if someone were using it that way, they were in a state of error...)




Patch:
diff --git a/bibliopixel/animation/__init__.py b/bibliopixel/animation/__init__.py
index 9faa07b..78df619 100644
--- a/bibliopixel/animation/__init__.py
+++ b/bibliopixel/animation/__init__.py
@@ -3,8 +3,15 @@ from . circle import BaseCircleAnim
 from . game import BaseGameAnim
 from . matrix import BaseMatrixAnim
 from . cube import BaseCubeAnim
-from . off import OffAnim
+from . off import Off
 from . sequence import Sequence
 from . strip import BaseStripAnim
 from . receiver import BaseReceiver
 from . tests import StripChannelTest, MatrixChannelTest, MatrixCalibrationTest
+
+
+# DEPRECATED
+strip_test = StripChannelTest
+matrix_calibration = MatrixCalibrationTest
+matrix_test = MatrixChannelTest
+from . off import OffAnim
diff --git a/bibliopixel/animation/off.py b/bibliopixel/animation/off.py
index 4b862ae..95764c9 100644
--- a/bibliopixel/animation/off.py
+++ b/bibliopixel/animation/off.py
@@ -1,7 +1,7 @@
 from . animation import BaseAnimation
 
 
-class OffAnim(BaseAnimation):
+class Off(BaseAnimation):
     """A trivial animation that turns all pixels in a layout off."""
 
     def __init__(self, layout, timeout=1, **kwds):
@@ -10,3 +10,7 @@ class OffAnim(BaseAnimation):
 
     def step(self, amt=1):
         self.layout.all_off()
+
+
+# DEPRECATED
+OffAnim = Off
diff --git a/bibliopixel/animation/receiver.py b/bibliopixel/animation/receiver.py
index 18fcb73..7029a1f 100644
--- a/bibliopixel/animation/receiver.py
+++ b/bibliopixel/animation/receiver.py
@@ -19,7 +19,7 @@ HOST_MAP = {
 }
 
 
-class BaseReceiver(BaseAnimation):
+class Receiver(BaseAnimation):
     free_run = True
 
     def __init__(self, layout, **kwds):
@@ -65,3 +65,7 @@ class BaseReceiver(BaseAnimation):
         if not self._stop_event.isSet():
             self._hold_for_data.wait()
             self._hold_for_data.clear()
+
+
+# DEPRECATED
+BaseReceiver = Receiver
diff --git a/bibliopixel/animation/remote/__init__.py b/bibliopixel/animation/remote/__init__.py
index e69de29..e88591e 100644
--- a/bibliopixel/animation/remote/__init__.py
+++ b/bibliopixel/animation/remote/__init__.py
@@ -0,0 +1,1 @@
+from . control import RemoteControl as Remote
diff --git a/bibliopixel/animation/remote/control.py b/bibliopixel/animation/remote/control.py
index 4851ccf..2078775 100644
--- a/bibliopixel/animation/remote/control.py
+++ b/bibliopixel/animation/remote/control.py
@@ -4,7 +4,7 @@ from .. animation import STATE
 from .. import collection
 from ... project import load
 from ... util import log
-from bibliopixel.animation.off import OffAnim
+from bibliopixel.animation.off import Off
 
 DEFAULT_OFF = 'OFF_ANIM'
 DEFAULT_ANIM_CONFIG = {
@@ -106,7 +106,7 @@ class RemoteControl(collection.Collection):
                 self.default = DEFAULT_OFF
 
         if self.default == DEFAULT_OFF:
-            off = OffAnim(self.layout)
+            off = Off(self.layout)
             off.set_runner({'threaded': True})
             self.animations.append(off)
             self.name_map[DEFAULT_OFF] = len(self.animations) - 1
diff --git a/bibliopixel/drivers/dummy.py b/bibliopixel/drivers/dummy.py
new file mode 100644
index 0000000..eabb117
--- /dev/null
+++ b/bibliopixel/drivers/dummy.py
@@ -0,0 +1,27 @@
+from . driver_base import DriverBase
+import time
+
+
+class Dummy(DriverBase):
+    """For Testing: Provides no ouput, just a valid interface"""
+
+    def __init__(self, num=1024, delay=0, **kwds):
+        """
+        Args
+            delay: time to wait in seconds to simulate actual hardware
+            interface time
+        """
+        super().__init__(num)
+        self._kwds = kwds
+        self._delay = delay
+
+    # Push new data to strand
+    def _compute_packet(self):
+        if self._delay > 0:
+            time.sleep(self._delay)
+        else:
+            pass
+
+
+# This is DEPRECATED.
+DriverDummy = Dummy
diff --git a/bibliopixel/drivers/dummy_driver.py b/bibliopixel/drivers/dummy_driver.py
index eabb117..868cf49 100644
--- a/bibliopixel/drivers/dummy_driver.py
+++ b/bibliopixel/drivers/dummy_driver.py
@@ -1,27 +1,4 @@
-from . driver_base import DriverBase
-import time
+# This whole file is DEPRECATED.
 
-
-class Dummy(DriverBase):
-    """For Testing: Provides no ouput, just a valid interface"""
-
-    def __init__(self, num=1024, delay=0, **kwds):
-        """
-        Args
-            delay: time to wait in seconds to simulate actual hardware
-            interface time
-        """
-        super().__init__(num)
-        self._kwds = kwds
-        self._delay = delay
-
-    # Push new data to strand
-    def _compute_packet(self):
-        if self._delay > 0:
-            time.sleep(self._delay)
-        else:
-            pass
-
-
-# This is DEPRECATED.
+from . dummy import Dummy
 DriverDummy = Dummy
diff --git a/bibliopixel/drivers/mirror.py b/bibliopixel/drivers/mirror.py
index 87306f1..023b8b9 100644
--- a/bibliopixel/drivers/mirror.py
+++ b/bibliopixel/drivers/mirror.py
@@ -4,7 +4,7 @@ from . driver_base import DriverBase
 import copy
 
 
-class MirrorDriver(DriverBase):
+class Mirror(DriverBase):
     @staticmethod
     def pre_recursion(desc):
         if 'drivers' not in desc:
@@ -42,3 +42,7 @@ class MirrorDriver(DriverBase):
         self.start = forward('start')
         self.sync = forward('sync')
         self.update_colors = forward('update_colors')
+
+
+# DEPRECATED
+MirrorDriver = Mirror
diff --git a/bibliopixel/drivers/simpixel.py b/bibliopixel/drivers/simpixel.py
new file mode 100644
index 0000000..30ac724
--- /dev/null
+++ b/bibliopixel/drivers/simpixel.py
@@ -0,0 +1,3 @@
+from . SimPixel.driver import SimPixel
+
+# TODO: rename SimPixel to simpixel, etc
diff --git a/bibliopixel/layout/matrix.py b/bibliopixel/layout/matrix.py
index f335c8a..8e8d529 100644
--- a/bibliopixel/layout/matrix.py
+++ b/bibliopixel/layout/matrix.py
@@ -1,7 +1,7 @@
 import math, threading, time
 
 from .. util import colors, log
-from . import matrix_drawing as matrix
+from . import matrix_drawing as md
 from . import font
 from . layout import MultiLayout
 from . geometry import make_matrix_coord_map_multi
@@ -215,13 +215,13 @@ class Matrix(MultiLayout):
         """
         Draw a circle in an RGB color, with center x0, y0 and radius r.
         """
-        matrix.draw_circle(self.set, x0, y0, r, color)
+        md.draw_circle(self.set, x0, y0, r, color)
 
     def fillCircle(self, x0, y0, r, color=None):
         """
         Draw a filled circle in an RGB color, with center x0, y0 and radius r.
         """
-        matrix.fill_circle(self.set, x0, y0, r, color)
+        md.fill_circle(self.set, x0, y0, r, color)
 
     def drawLine(self, x0, y0, x1, y1, color=None, colorFunc=None, aa=False):
         """
@@ -233,7 +233,7 @@ class Matrix(MultiLayout):
             Xiaolin Wu's algorithm
 
         """
-        matrix.draw_line(self.set, x0, y0, x1, y1, color, colorFunc, aa)
+        md.draw_line(self.set, x0, y0, x1, y1, color, colorFunc, aa)
 
     # Bresenham's algorithm
     def bresenham_line(self, x0, y0, x1, y1, color=None, colorFunc=None):
@@ -242,7 +242,7 @@ class Matrix(MultiLayout):
 
         Will draw beyond matrix bounds.
         """
-        matrix.bresenham_line(self.set, x0, y0, x1, y1, color, colorFunc)
+        md.bresenham_line(self.set, x0, y0, x1, y1, color, colorFunc)
 
     # Xiaolin Wu's Line Algorithm
     def wu_line(self, x0, y0, x1, y1, color=None, colorFunc=None):
@@ -254,7 +254,7 @@ class Matrix(MultiLayout):
         :param aa: if True, use Bresenham's algorithm for line drawing; otherwise use
             Xiaolin Wu's algorithm
         """
-        matrix.wu_line(self.set, x0, y0, x1, y1, color, colorFunc)
+        md.wu_line(self.set, x0, y0, x1, y1, color, colorFunc)
 
     def drawRect(self, x, y, w, h, color=None, aa=False):
         """
@@ -263,7 +263,7 @@ class Matrix(MultiLayout):
         :param aa: if True, use Bresenham's algorithm for line drawing; otherwise use
             Xiaolin Wu's algorithm
         """
-        matrix.draw_rect(self.set, x, y, w, h, color, aa)
+        md.draw_rect(self.set, x, y, w, h, color, aa)
 
     def fillRect(self, x, y, w, h, color=None, aa=False):
         """
@@ -273,11 +273,11 @@ class Matrix(MultiLayout):
         :param aa: if True, use Bresenham's algorithm for line drawing; otherwise use
             Xiaolin Wu's algorithm
         """
-        matrix.fill_rect(self.set, x, y, w, h, color, aa)
+        md.fill_rect(self.set, x, y, w, h, color, aa)
 
     def fillScreen(self, color=None):
         """Fill the matrix with the given RGB color"""
-        matrix.fill_rect(self.set, 0, 0, self.width, self.height, color)
+        md.fill_rect(self.set, 0, 0, self.width, self.height, color)
 
     def drawRoundRect(self, x, y, w, h, r, color=None, aa=False):
         """
@@ -288,7 +288,7 @@ class Matrix(MultiLayout):
             Xiaolin Wu's algorithm
 
         """
-        matrix.draw_round_rect(self.set, x, y, w, h, r, color, aa)
+        md.draw_round_rect(self.set, x, y, w, h, r, color, aa)
 
     def fillRoundRect(self, x, y, w, h, r, color=None, aa=False):
         """
@@ -299,7 +299,7 @@ class Matrix(MultiLayout):
             Xiaolin Wu's algorithm
 
         """
-        matrix.fill_round_rect(self.set, x, y, w, h, r, color, aa)
+        md.fill_round_rect(self.set, x, y, w, h, r, color, aa)
 
     def drawTriangle(self, x0, y0, x1, y1, x2, y2, color=None, aa=False):
         """
@@ -309,7 +309,7 @@ class Matrix(MultiLayout):
             Xiaolin Wu's algorithm
 
         """
-        matrix.draw_triangle(self.set, x0, y0, x1, y1, x2, y2, color, aa)
+        md.draw_triangle(self.set, x0, y0, x1, y1, x2, y2, color, aa)
 
     def fillTriangle(self, x0, y0, x1, y1, x2, y2, color=None, aa=False):
         """
@@ -319,7 +319,7 @@ class Matrix(MultiLayout):
             Xiaolin Wu's algorithm
 
         """
-        matrix.fill_triangle(self.set, x0, y0, x1, y1, x2, y2, color, aa)
+        md.fill_triangle(self.set, x0, y0, x1, y1, x2, y2, color, aa)
 
     fillTrangle = fillTriangle  # DEPRECATED!
 
@@ -328,8 +328,8 @@ class Matrix(MultiLayout):
         """
         Draw a single character c at at (x, y) in an RGB color.
         """
-        matrix.draw_char(self.fonts, self.set, self.width, self.height,
-                         x, y, c, color, bg, aa, font, font_scale)
+        md.draw_char(self.fonts, self.set, self.width, self.height,
+                     x, y, c, color, bg, aa, font, font_scale)
 
     def drawText(self, text, x=0, y=0, color=None,
                  bg=colors.Off, aa=False, font=font.default_font, font_scale=1):
@@ -342,8 +342,8 @@ class Matrix(MultiLayout):
             rectangle for the text block.
 
         """
-        matrix.draw_text(self.fonts, self.set, text, self.width, self.height,
-                         x, y, color, bg, aa, font, font_scale)
+        md.draw_text(self.fonts, self.set, text, self.width, self.height,
+                     x, y, color, bg, aa, font, font_scale)
 
 
 # This is DEPRECATED
diff --git a/bibliopixel/project/alias_lists.py b/bibliopixel/project/alias_lists.py
index 2ed1fc7..d212d8c 100644
--- a/bibliopixel/project/alias_lists.py
+++ b/bibliopixel/project/alias_lists.py
@@ -4,41 +4,16 @@ from .. util import log
 PROJECT_ALIASES = {}
 
 BUILTIN_ALIASES = {
-    # drivers
     'apa102': 'bibliopixel.drivers.SPI.APA102.APA102',
-    'sk9822': 'bibliopixel.drivers.SPI.APA102.APA102',
-    'dummy': 'bibliopixel.drivers.dummy_driver.Dummy',
-    'hue': 'bibliopixel.drivers.hue.Hue',
     'lpd8806': 'bibliopixel.drivers.SPI.LPD8806.LPD8806',
-    'mirror': 'bibliopixel.drivers.mirror.MirrorDriver',
-    'network': 'bibliopixel.drivers.network.Network',
-    'network_udp': 'bibliopixel.drivers.network_udp.NetworkUDP',
+    'pi_ws281x': 'bibliopixel.drivers.PiWS281X.PiWS281X',
     'serial': 'bibliopixel.drivers.serial.Serial',
-    'simpixel': 'bibliopixel.drivers.SimPixel.SimPixel',
-    'ws281x': 'bibliopixel.drivers.SPI.WS281X.WS281X',
-    'ws2801': 'bibliopixel.drivers.SPI.WS2801.WS2801',
+    'sk9822': 'bibliopixel.drivers.SPI.APA102.APA102',
     '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',
+    'ws2801': 'bibliopixel.drivers.SPI.WS2801.WS2801',
+    'ws281x': 'bibliopixel.drivers.SPI.WS281X.WS281X',
 
-    # animations
-    'off': 'bibliopixel.animation.off.OffAnim',
-    'feedback': 'bibliopixel.animation.feedback.Feedback',
-    'matrix_calibration': 'bibliopixel.animation.tests.MatrixCalibrationTest',
-    'matrix_test': 'bibliopixel.animation.tests.MatrixChannelTest',
-    'mixer': 'bibliopixel.animation.mixer.Mixer',
-    'split': 'bibliopixel.animation.split.Split',
-    'receiver': 'bibliopixel.animation.receiver.BaseReceiver',
     'reprocess': 'bibliopixel.animation.reprocess.reprocess.Reprocess',
-    'remote': 'bibliopixel.animation.remote.control.RemoteControl',
-    'sequence': 'bibliopixel.animation.Sequence',
-    'strip_test': 'bibliopixel.animation.tests.StripChannelTest',
 }
 
 


Test patch:
diff --git a/test/bibliopixel/project/aliases_test.py b/test/bibliopixel/project/aliases_test.py
index 82f46ca..551bb14 100644
--- a/test/bibliopixel/project/aliases_test.py
+++ b/test/bibliopixel/project/aliases_test.py
@@ -13,7 +13,7 @@ class AliasTest(unittest.TestCase):
 
     def test_resolve(self):
         self.assertEqual(
-            aliases.resolve('off'), 'bibliopixel.animation.off.OffAnim')
+            aliases.resolve('serial'), 'bibliopixel.drivers.serial.Serial')
         self.assertEquals(aliases.resolve('foo'), 'foo')
 
         with patch(foo='bar'):
@@ -48,3 +48,32 @@ class AliasTest(unittest.TestCase):
         additional = {'foo': 'bar', 'remote': 'distance'}
         self.assertEqual(aliases.resolve('foo', additional), 'bar')
         self.assertEqual(aliases.resolve('remote', additional), 'distance')
+
+    def test_not_needed(self):
+        just_one = ''
+        failed, not_equal = [], []
+
+        for alias, path in alias_lists.BUILTIN_ALIASES.items():
+            if just_one and alias != just_one:
+                continue
+            python_path = '.'.join(path.split('.', 2)[:2])
+            expected = importer.import_symbol(path)
+            try:
+                actual = importer.import_symbol(alias, python_path=python_path)
+            except:
+                failed.append(alias)
+                if just_one:
+                    raise
+            else:
+                if actual is not expected:
+                    not_equal.append(alias)
+
+        self.assertEquals([sorted(failed), sorted(not_equal)],
+                          [sorted(FAILED), sorted(NOT_EQUAL)])
+
+
+# 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'
diff --git a/test/bibliopixel/project/bad_project_test.py b/test/bibliopixel/project/bad_project_test.py
index 5c6fbe4..b70e6a4 100644
--- a/test/bibliopixel/project/bad_project_test.py
+++ b/test/bibliopixel/project/bad_project_test.py
@@ -102,7 +102,7 @@ class BadProjectTest(unittest.TestCase):
         animation = make(BAD_ANIMATION_ATTRIBUTE, run_start=False)
         self.assertEquals(
             animation.exception.args,
-            ('Unknown attribute for animation OffAnim: "bad_attribute"',))
+            ('Unknown attribute for animation Off: "bad_attribute"',))
 
     def test_bad_run_attribute(self):
         animation = make(BAD_RUN_ATTRIBUTE, run_start=False)