Repository: ManiacalLabs/BiblioPixel (python)

Issue:
Better handling of "failed" lists in Yaml input.
Example here: https://groups.google.com/forum/#!topic/maniacal-labs-users/HIPLxEKaQeI

I have made this mistake several times in projects - written

    colors: red, green, blue

which results in `{'colors': 'red, green, blue'}` - the value is one string! when I meant 

    colors: [red, green, blue]

which results in `{'colors': ['red', 'green', 'blue']}`

Now, `bp` isn't doing anything "wrong" - the user is "making a mistake" in the .yml, but it's really Yaml that is encouraging this mistake.

...but we can _recover_ from this "mistake"

I'm not going to attempt a general fix - I see this happening in these cases:

    colors: red, green, blue
    shape: 2, 3
    shape: (2, 3)

so I'm going to change the `colors` Field type parser and add a new shape Field parser.


Pull request description:
Better handling of "failed" Yaml entry of lists (fix #1099)

Patch:
diff --git a/bibliopixel/colors/make.py b/bibliopixel/colors/make.py
index 4455632..9585ee9 100644
--- a/bibliopixel/colors/make.py
+++ b/bibliopixel/colors/make.py
@@ -1,4 +1,4 @@
-import functools, numbers
+import functools, numbers, re
 from . import names
 from . import palette
 
@@ -109,14 +109,30 @@ def _colors(colors=None, **kwds):
 
 
 def colors_no_palette(colors=None, **kwds):
-    """Return a Palette but don't take into account Pallet Names."""
+    """Return a Palette but don't take into account Pallete Names."""
     if isinstance(colors, str):
-        colors = colors.split(',')
+        colors = _split_colors(colors)
+    else:
+        colors = to_triplets(colors or ())
 
-    colors = (color(c) for c in to_triplets(colors or ()))
+    colors = (color(c) for c in colors or ())
     return palette.Palette(colors, **kwds)
 
 
+def _split_colors(colors):
+    brackets = False
+    for part in _COLOR_SPLIT_RE.split(colors):
+        if brackets:
+            yield part
+        else:
+            part = [p.strip() for p in part.split(',')]
+            part = [p for p in part if p]
+            yield from part
+        brackets = not brackets
+
+
+_COLOR_SPLIT_RE = re.compile(r'( [([] .*? [])] )', re.X)
+
 _COLOR_USAGE = """
 A Color can be initialized with:
 
@@ -124,7 +140,7 @@ A Color can be initialized with:
 * A single number which represents a brightness/gray level: 0, 255, 127
 * A string:  "red", "yellow", "gold" naming a color from ...colors.COLORS.
 
-All numbers must be in the range [0, 256) - 0 <= x < 256"""
+All numbers should be in the range [0, 256) - 0 <= x <= 255"""
 
 _COLORS_USAGE = """
 Colors is a list of colors.  Each color can be:
@@ -133,4 +149,4 @@ Colors is a list of colors.  Each color can be:
 * A single number which represents a brightness/gray level: 0, 255, 127
 * A string:  "red", "yellow", "gold" naming a color from ...colors.COLORS.
 
-All numbers must be in the range [0, 256) - 0 <= x < 256"""
+All numbers must be in the range [0, 256) - 0 <= x <= 255"""
diff --git a/bibliopixel/colors/names.py b/bibliopixel/colors/names.py
index 6704d4d..f8da215 100644
--- a/bibliopixel/colors/names.py
+++ b/bibliopixel/colors/names.py
@@ -20,6 +20,8 @@ def name_to_color(name):
         if ',' in name:
             if name.startswith('(') and name.endswith(')'):
                 name = name[1:-1]
+            if name.startswith('[') and name.endswith(']'):
+                name = name[1:-1]
 
             r, g, b = name.split(',')
             return _from_number(r), _from_number(g), _from_number(b)
diff --git a/bibliopixel/project/fill.py b/bibliopixel/project/fill.py
index 89313c2..9b80735 100644
--- a/bibliopixel/project/fill.py
+++ b/bibliopixel/project/fill.py
@@ -120,7 +120,15 @@ def _fill_shape(desc):
 
     if isinstance(shape, int):
         shape = [shape]
-    elif not isinstance(shape, list):
+    elif isinstance(shape, str):
+        shape = shape.strip()
+        if shape.startswith('(') and shape.endswith(')'):
+            shape = shape[1:-1]
+        try:
+            shape = [int(s) for s in shape.split(',')]
+        except:
+            raise ValueError('Cannot parse shape %s' % shape)
+    elif not isinstance(shape, (list, tuple)):
         raise ValueError('`shape` must be a number or a list, was "%s"' % shape)
 
     ldesc = construct.to_type_constructor(desc.get('layout') or {},


Test patch:
diff --git a/test/bibliopixel/colors/make_test.py b/test/bibliopixel/colors/make_test.py
new file mode 100644
index 0000000..761f43b
--- /dev/null
+++ b/test/bibliopixel/colors/make_test.py
@@ -0,0 +1,41 @@
+import unittest
+from fractions import Fraction
+
+from bibliopixel.colors import make, COLORS
+
+
+class MakeTest(unittest.TestCase):
+    def test_color(self):
+        self.assertEqual(make.color('red'), COLORS.red)
+
+    def test_split_colors1(self):
+        test = 'red, green, [25, 81, 100], yellow'
+        actual = list(make._split_colors(test))
+        expected = ['red', 'green', '[25, 81, 100]', 'yellow']
+        self.assertEqual(actual, expected)
+
+    def test_split_colors2(self):
+        test = '[12, 12, 12], red, green, [25, 81, 100], yellow, (3, 4, 5)'
+        actual = list(make._split_colors(test))
+        expected = ['[12, 12, 12]', 'red', 'green', '[25, 81, 100]',
+                    'yellow', '(3, 4, 5)']
+        self.assertEqual(actual, expected)
+
+    def test_colors0(self):
+        test = 'red, green, yellow'
+        actual = list(make.colors(test))
+        expected = [COLORS.red, COLORS.green, COLORS.yellow]
+        self.assertEqual(actual, expected)
+
+    def test_colors1(self):
+        test = 'red, green, [25, 81, 100], yellow'
+        actual = list(make.colors(test))
+        expected = [COLORS.red, COLORS.green, (25, 81, 100), COLORS.yellow]
+        self.assertEqual(actual, expected)
+
+    def test_colors2(self):
+        test = '[12, 12, 12], red, green, [25, 81, 100], yellow, (3, 4, 5)'
+        actual = list(make.colors(test))
+        expected = [(12, 12, 12), COLORS.red, COLORS.green, (25, 81, 100),
+                    COLORS.yellow, (3, 4, 5)]
+        self.assertEqual(actual, expected)
diff --git a/test/bibliopixel/project/fill_test.py b/test/bibliopixel/project/fill_test.py
index f022afa..727198f 100644
--- a/test/bibliopixel/project/fill_test.py
+++ b/test/bibliopixel/project/fill_test.py
@@ -4,6 +4,7 @@ from bibliopixel.animation.sequence import Sequence
 from bibliopixel.animation.matrix import Matrix
 from bibliopixel.animation.off import Off
 from bibliopixel.layout import cube, matrix, strip
+from bibliopixel.util import data_file
 
 BASE = {
     'animation': {
@@ -104,3 +105,13 @@ class FillTest(unittest.TestCase):
             'layout': {'datatype': cube.Cube, 'x': 2, 'y': 3, 'z': 7},
         }
         self.assertEqual(actual, dict(BASE, **expected))
+
+    def test_wrong_shape(self):
+        for shape in '(2, 3, 7)', '2, 3, 7':
+            source = {'shape': shape}
+            actual = fill_before(source)
+            expected = {
+                'drivers': [{'num': 42, 'typename': 'simpixel'}],
+                'layout': {'datatype': cube.Cube, 'x': 2, 'y': 3, 'z': 7},
+            }
+            self.assertEqual(actual, dict(BASE, **expected))
diff --git a/test/bibliopixel/project/types/color_test.py b/test/bibliopixel/project/types/color_test.py
index da2f16b..b23cbbf 100644
--- a/test/bibliopixel/project/types/color_test.py
+++ b/test/bibliopixel/project/types/color_test.py
@@ -23,6 +23,13 @@ class ColorTypesTest(TypesBaseTest):
             self.make('color', c, c)
             self.make('color', str(c), c)
 
+    def test_edge_cases(self):
+        a = self.make('color', '[0, 0, 0]')
+        b = self.make('color', '(0, 0, 0)')
+        expected = {'color': (0, 0, 0)}
+        self.assertEqual(a, expected)
+        self.assertEqual(b, expected)
+
     def test_errors(self):
         with self.assertRaises(ValueError):
             self.make('color', -1)
@@ -33,9 +40,6 @@ class ColorTypesTest(TypesBaseTest):
         with self.assertRaises(ValueError):
             self.make('color', '')
 
-        with self.assertRaises(ValueError):
-            self.make('color', '[0, 0, 0]')
-
         with self.assertRaises(ValueError):
             self.make('color', 'dog')