Repository: ManiacalLabs/BiblioPixel
Pull request #593 (2018-01-26 17:46:27)

Tweaks
A grab-bag of independent changes, mostly spun off from the general controllers branch. 

adammhaile: +1

On Fri, Jan 26, 2018 at 12:46 PM, Tom Swirly <notifications@github.com>
wrote:

> A grab-bag of independent changes, mostly spun off from the general
> controllers branch.
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
>   https://github.com/ManiacalLabs/BiblioPixel/pull/593
> Commit Summary
>
>    - Unflatten unpacks flat arrays into deep ones.
>    - Collect a little more information in midi controller
>    - Fix a typo in extractor.py
>    - Allow addresses to have leading or trailing dots
>    - Change a parameter name in collection.Wrapper
>    - BaseAnimation.step() now does nothing
>
> File Changes
>
>    - *M* bibliopixel/animation/animation.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-0>
>    (2)
>    - *M* bibliopixel/animation/collection.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-1>
>    (4)
>    - *M* bibliopixel/animation/feedback.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-2>
>    (4)
>    - *M* bibliopixel/controllers/address.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-3>
>    (36)
>    - *M* bibliopixel/controllers/extractor.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-4>
>    (2)
>    - *M* bibliopixel/controllers/midi.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-5>
>    (33)
>    - *M* bibliopixel/util/flatten.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-6>
>    (37)
>    - *M* test/bibliopixel/controllers/address_test.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-7>
>    (4)
>    - *M* test/bibliopixel/controllers/midi_test.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-8>
>    (19)
>    - *M* test/bibliopixel/util/flatten_test.py
>    <https://github.com/ManiacalLabs/BiblioPixel/pull/593/files#diff-9>
>    (17)
>
> Patch Links:
>
>    - https://github.com/ManiacalLabs/BiblioPixel/pull/593.patch
>    - https://github.com/ManiacalLabs/BiblioPixel/pull/593.diff
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/ManiacalLabs/BiblioPixel/pull/593>, or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AA6a6sB4T51TIO71_wXHuIkNbSUGYddHks5tOg9zgaJpZM4RupPs>
> .
>


Patch:
diff --git a/bibliopixel/animation/animation.py b/bibliopixel/animation/animation.py
index 235be8a..6c97d4a 100644
--- a/bibliopixel/animation/animation.py
+++ b/bibliopixel/animation/animation.py
@@ -76,7 +76,7 @@ class BaseAnimation(object):
         return self.__class__.__name__
 
     def step(self, amt=1):
-        raise RuntimeError("Base class step() called. This shouldn't happen")
+        pass
 
     def cleanup(self, clean_layout=True):
         # if current thread is animation thread this was called
diff --git a/bibliopixel/animation/collection.py b/bibliopixel/animation/collection.py
index be2a90e..e33bfb4 100644
--- a/bibliopixel/animation/collection.py
+++ b/bibliopixel/animation/collection.py
@@ -90,8 +90,8 @@ class Parallel(Collection):
 
 
 class Wrapper(Collection):
-    def __init__(self, *args, source, **kwds):
-        super()(*args, animations=[source], **kwds)
+    def __init__(self, *args, animation, **kwds):
+        super()(*args, animations=[animation], **kwds)
         self.animation = self.animations[0]
 
     def step(self, amt=1):
diff --git a/bibliopixel/animation/feedback.py b/bibliopixel/animation/feedback.py
index d9b0901..8d9f13d 100644
--- a/bibliopixel/animation/feedback.py
+++ b/bibliopixel/animation/feedback.py
@@ -4,9 +4,9 @@ from .. util import color_list
 
 
 class Feedback(Wrapper):
-    def __init__(self, *args, source,
+    def __init__(self, *args, animation,
                  master=1, inputs=None, outputs=None, **kwds):
-        super().__init__(*args, source=source, **kwds)
+        super().__init__(*args, animation=animation, **kwds)
         self.master = master
 
         inputs = inputs or [0]
diff --git a/bibliopixel/controllers/address.py b/bibliopixel/controllers/address.py
index 89e4a5c..aebf7de 100644
--- a/bibliopixel/controllers/address.py
+++ b/bibliopixel/controllers/address.py
@@ -1,12 +1,14 @@
 """
-An address identifies the location data within a Python object.
+An address identifies how to get or set a piece of data within a Python object
+using attributes and indexing.
 
-An address looks like
+An address description is a string looking like:
 
-    foo.bar[32][5][baz].bang
+    .foo.bar[32][5][baz].bang
 
-That address would mean "given some object x, the value of
-x.foo.bar[32][5]['baz'].bang".
+which would mean
+
+"given an object x, the value x.foo.bar[32][5]['baz'].bang".
 
 Addresses are divided into "segments".  Segments in brackets are indexes or keys;
 otherwise they are attributes.  In the example above, the segments are
@@ -30,24 +32,32 @@ class Address:
         def __init__(self, name):
             self.name = name
 
+        def set(self, x, *value):
+            return self._set(x, (value[0] if len(value) == 1 else value))
+
     class Attribute(Segment):
         def get(self, x):
             return getattr(x, self.name)
 
-        def set(self, x, value):
+        def _set(self, x, value):
             return setattr(x, self.name, value)
 
     class Index(Segment):
         def get(self, x):
             return x[self.name]
 
-        def set(self, x, value):
+        def _set(self, x, value):
             x[self.name] = value
 
     def __init__(self, s):
-        def generate():
-            # First split on dots, then use [ and ] to split out indices.
-            for i, part in enumerate(s.strip().split('.')):
+        def generate(s):
+            # Split on dots, then use [ and ] to split out indices
+            s = s.strip()
+            while s.startswith('.'):
+                s = s[1:]
+            while s.endswith('.'):
+                s = s[:-1]
+            for i, part in enumerate(s.split('.')):
                 head, *rest = part.split('[')
 
                 # Suppose our address was 'xxx[yyy][zzz]'
@@ -73,7 +83,7 @@ class Address:
                     yield Address.Index(index)
 
         try:
-            self.address = list(generate())
+            self.address = list(generate(s))
         except:
             raise ValueError('%s is not a legal address' % s)
 
@@ -89,7 +99,7 @@ class Address:
     def get(self, x):
         return self._get(x, self.address)
 
-    def set(self, x, value):
+    def set(self, x, *value):
         *first, last = self.address
         parent = self._get(x, first)
-        last.set(parent, value)
+        last.set(parent, *value)
diff --git a/bibliopixel/controllers/extractor.py b/bibliopixel/controllers/extractor.py
index 150675f..15bd23b 100644
--- a/bibliopixel/controllers/extractor.py
+++ b/bibliopixel/controllers/extractor.py
@@ -3,7 +3,7 @@ import collections
 
 class Extractor:
     """
-    MessageToDict is a class that extracts and normalizes values from
+    Extractor is a class that extracts and normalizes values from
     incoming message dictionaries into ordered dictionaries based on the
     `type` key of each message.
     """
diff --git a/bibliopixel/controllers/midi.py b/bibliopixel/controllers/midi.py
index f863654..15400c2 100644
--- a/bibliopixel/controllers/midi.py
+++ b/bibliopixel/controllers/midi.py
@@ -22,12 +22,18 @@ class Midi(control_source.ExtractedSource):
     # mido MIDI messages.
     # There are more mido message types that we haven't used yet.
     KEYS_BY_TYPE = {
-        'aftertouch': ('channel', 'type', 'value'),
-        'control_change': ('channel', 'type', 'control', 'value'),
-        'note_off': ('channel', 'type', 'note', 'velocity'),
-        'note_on': ('channel', 'type', 'note', 'velocity'),
-        'pitchwheel': ('channel', 'type', 'pitch'),
-        'program_change': ('channel', 'type', 'program'),
+        'aftertouch': (
+            'port', 'channel', 'type', 'value'),
+        'control_change': (
+            'port', 'channel', 'type', 'control', 'value'),
+        'note_off': (
+            'port', 'channel', 'type', 'note', 'velocity'),
+        'note_on': (
+            'port', 'channel', 'type', 'note', 'velocity'),
+        'pitchwheel': (
+            'port', 'channel', 'type', 'pitch'),
+        'program_change': (
+            'port', 'channel', 'type', 'program'),
     }
 
     # Some numeric fields (channel, control, program and note) don't get
@@ -39,7 +45,16 @@ class Midi(control_source.ExtractedSource):
         'velocity': lambda x: fractions.Fraction(x) / 127,
     }
 
+    def __init__(self, omit=('port', 'channel'), use_note_off=False, **kwds):
+        super().__init__(omit=omit, **kwds)
+        self.use_note_off = use_note_off
+
     def __iter__(self):
-        inputs = mido.get_input_names()
-        for msg in mido.ports.MultiPort(mido.open_input(i) for i in inputs):
-            yield vars(msg)
+        ports = [mido.open_input(i) for i in mido.get_input_names()]
+
+        for port, msg in mido.ports.MultiPort(ports, yield_ports=True):
+            mdict = dict(vars(msg), port=port)
+            if self.use_note_off or msg.type != 'note_off':
+                yield mdict
+            else:
+                yield dict(mdict, type='note_on', velocity=0)
diff --git a/bibliopixel/util/flatten.py b/bibliopixel/util/flatten.py
index 9c5a704..93bb23e 100644
--- a/bibliopixel/util/flatten.py
+++ b/bibliopixel/util/flatten.py
@@ -42,3 +42,40 @@ def flatten(master):
 
     recurse(master)
     return result
+
+
+def unflatten(master):
+    """
+    Unflattens a single-level dictionary a multilevel into one so that::
+
+        {'foo.bar.a': 1,
+         'foo.bar.b': True,
+         'foo.bar.a': 1,
+         }
+
+    would become::
+
+        {'foo':
+            {'bar':
+               {
+                   'a': 1,
+                   'b': True,
+                   'c': 'hello',
+                },
+            },
+        }
+    """
+    result = {}
+
+    for k, v in master.items():
+        *first, last = k.split('.')
+        r = result
+        for i in first:
+            r = r.setdefault(i, {})
+        r[last] = v
+
+    return result
+
+
+def canonical(master):
+    return unflatten(flatten(master))


Test patch:
diff --git a/test/bibliopixel/controllers/address_test.py b/test/bibliopixel/controllers/address_test.py
index 4cd44a8..e430e19 100644
--- a/test/bibliopixel/controllers/address_test.py
+++ b/test/bibliopixel/controllers/address_test.py
@@ -19,7 +19,7 @@ class AddressTest(unittest.TestCase):
         self.assertIs(address.get(self), 'bang')
 
     def test_attrib_error(self):
-        address = Address('attr')
+        address = Address('.attr')
         with self.assertRaises(AttributeError):
             address.get(AddressTest)
 
@@ -50,7 +50,7 @@ class AddressTest(unittest.TestCase):
         self.attr2 = self
         self.attr3 = 'bingo'
 
-        address = Address('attr1[0][test][1][heck].attr2.attr3')
+        address = Address('.attr1[0][test][1][heck].attr2.attr3')
         self.assertEqual(len(address.address), 7)
         self.assertEqual(address.get(self), 'bingo')
         address.set(self, 'bang')
diff --git a/test/bibliopixel/controllers/midi_test.py b/test/bibliopixel/controllers/midi_test.py
index 678f11c..aa59043 100644
--- a/test/bibliopixel/controllers/midi_test.py
+++ b/test/bibliopixel/controllers/midi_test.py
@@ -14,8 +14,14 @@ OTHER = Namespace(type='other', channel=32, thing='stuff')
 
 class FakeMido:
     def __init__(self, msgs):
+        def multi_port(x, yield_ports=False):
+            if yield_ports:
+                return (('fake_port', i) for i in msgs)
+            else:
+                return msgs
+
         self.ports = self
-        self.ports.MultiPort = lambda x: msgs
+        self.ports.MultiPort = multi_port
 
     def get_input_names(self):
         return ['one', 'two']
@@ -33,18 +39,23 @@ class MidiTest(unittest.TestCase):
             m.start()
             m.thread.join()
             actual = [list(i.items()) for i in actual]
+            if actual != expected:
+                print('FAIL')
+                print(actual)
+                print(expected)
             self.assertEquals(actual, expected)
 
     def test_one(self):
         expected = [
+            ('port', 'fake_port'),
             ('channel', 1),
             ('type', 'note_on'),
             ('note', 32),
             ('velocity', fractions.Fraction(96) / 127)]
 
-        self.run_test([C3], [expected])
-        self.run_test([C3, C3], [expected, expected])
-        self.run_test([C3, C3], [expected[1:], expected[1:]], omit='channel')
+        self.run_test([C3], [expected[2:]])
+        self.run_test([C3, C3], [expected[2:], expected[2:]])
+        self.run_test([C3], [expected], omit=None)
 
     def test_accept(self):
         accept = {'channel': 2, 'type': 'control_change', 'control': 2}
diff --git a/test/bibliopixel/util/flatten_test.py b/test/bibliopixel/util/flatten_test.py
index a1f2024..56c5254 100644
--- a/test/bibliopixel/util/flatten_test.py
+++ b/test/bibliopixel/util/flatten_test.py
@@ -1,28 +1,41 @@
 import unittest
 
-from bibliopixel.util.flatten import flatten
+from bibliopixel.util.flatten import canonical, flatten, unflatten
 
 
 class FlattenTest(unittest.TestCase):
     def assert_flatten(self, before, after=None):
         self.assertEqual(flatten(before), after or before)
         self.assertEqual(flatten(flatten(before)), after or before)
+        after and self.assertEqual(canonical(before), canonical(after))
+
+    def assert_unflatten(self, before, after=None):
+        self.assertEqual(before, unflatten(after or before))
+        self.assertEqual(before, unflatten(unflatten(after or before)))
+        after and self.assertEqual(canonical(before), canonical(after))
 
     def test_trivial(self):
         self.assert_flatten({})
 
     def test_simple(self):
-        self.assert_flatten({'a': 1, 'c.d': True})
+        before = {'a': 1, 'c': {'d': True}}
+        after = {'a': 1, 'c.d': True}
+        self.assert_flatten(before, after)
+        self.assert_unflatten(before, after)
 
     def test_three_levels(self):
         before = {'foo': {'bar': {'a': 1, 'b': True, 'c': 'X'}}}
         after = {'foo.bar.a': 1, 'foo.bar.b': True, 'foo.bar.c': 'X'}
         self.assert_flatten(before, after)
+        self.assert_unflatten(before, after)
 
     def test_mix(self):
         before = {'foo': {'bar': {'a': 1}, 'bar.c': 'X'}, 'foo.bar.b': True}
         after = {'foo.bar.a': 1, 'foo.bar.b': True, 'foo.bar.c': 'X'}
+        canon = {'foo': {'bar': {'a': 1, 'b': True, 'c': 'X'}}}
         self.assert_flatten(before, after)
+        self.assert_unflatten(canon, after)
+        self.assertEqual(canonical(before), canon)
 
     def test_error(self):
         with self.assertRaises(ValueError):