Repository: ManiacalLabs/BiblioPixel
Pull request #806 (2018-05-16 17:49:27)

Improve some errors


Patch:
diff --git a/bibliopixel/drivers/network_receiver.py b/bibliopixel/drivers/network_receiver.py
index 7505644..dd5b6be 100644
--- a/bibliopixel/drivers/network_receiver.py
+++ b/bibliopixel/drivers/network_receiver.py
@@ -1,14 +1,8 @@
-import threading
-import os
+import os, threading
 
-try:
-    import SocketServer
-except:
-    import socketserver as SocketServer
+import socketserver as SocketServer
 from .. drivers.return_codes import RETURN_CODES
 from . network import CMDTYPE
-
-os.sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 from .. util import log
 
 
diff --git a/bibliopixel/project/construct.py b/bibliopixel/project/construct.py
index 0a4e32f..a084488 100644
--- a/bibliopixel/project/construct.py
+++ b/bibliopixel/project/construct.py
@@ -17,7 +17,15 @@ def to_type(d):
     fn = load.load_if_filename(d)
     if fn:
         return fn
-    return {'typename': d} if isinstance(d, str) else copy.deepcopy(d)
+    if isinstance(d, str):
+        return {'typename': d}
+
+    result = copy.deepcopy(d)
+
+    # 'datatype' always overrides 'typename'
+    if 'datatype' in result:
+        result.pop('typename', None)
+    return result
 
 
 def to_type_constructor(value, python_path=None):
@@ -46,6 +54,7 @@ def to_type_constructor(value, python_path=None):
         try:
             value['datatype'] = importer.import_symbol(
                 r, python_path=python_path)
+            del value['typename']
         except Exception as e:
             value['_exception'] = e
 
diff --git a/bibliopixel/project/merge.py b/bibliopixel/project/merge.py
index dab9034..e367e9d 100644
--- a/bibliopixel/project/merge.py
+++ b/bibliopixel/project/merge.py
@@ -16,7 +16,7 @@ DEFAULT_PROJECT = collections.OrderedDict((
     ('shape', ()),
     ('layout', {}),
     ('run', {}),
-    ('animation', {}),
+    ('animation', {'typename': 'animation'}),
     ('controls', []),
 ))
 PROJECT_SECTIONS = tuple(DEFAULT_PROJECT.keys())
@@ -26,6 +26,7 @@ NOT_MERGEABLE = (
     'typename')
 
 SECTION_ISNT_DICT_ERROR = 'Project section "%s" is %s, should be dictionary'
+UNKNOWN_SECTION_ERROR = 'There is no Project section named "%s"'
 
 
 def merge(*projects):
@@ -40,6 +41,9 @@ def merge(*projects):
                 result[name] = section
                 continue
 
+            if name not in DEFAULT_PROJECT:
+                raise ValueError(UNKNOWN_SECTION_ERROR % name)
+
             if section and not isinstance(section, (dict, str)):
                 cname = section.__class__.__name__
                 raise ValueError(SECTION_ISNT_DICT_ERROR % (name, cname))
@@ -52,5 +56,7 @@ def merge(*projects):
                     section['run'] = adesc.get('run', {})
 
             result.setdefault(name, {}).update(construct.to_type(section))
+            if 'datatype' in result[name]:
+                result[name].pop('typename', None)
 
     return result
diff --git a/bibliopixel/project/project.py b/bibliopixel/project/project.py
index 6ec62b2..b3121cd 100644
--- a/bibliopixel/project/project.py
+++ b/bibliopixel/project/project.py
@@ -4,6 +4,7 @@ from . import (
 from .. util import exception, json, log
 
 EVENT_QUEUE_MAXSIZE = 1000
+NO_DATATYPE_ERROR = 'No "datatype" field in section "%s"'
 
 
 class Project:
@@ -13,7 +14,11 @@ class Project:
     def pre_recursion(desc):
         return cleanup.cleanup(desc)
 
-    def construct_child(self, datatype, typename=None, **kwds):
+    def construct_child(self, section_name, datatype=None, typename=None,
+                        **kwds):
+        if not datatype:
+            raise ValueError(NO_DATATYPE_ERROR % section_name)
+
         construct = getattr(datatype, 'construct', None)
         if construct:
             return construct(self, **kwds)
@@ -26,13 +31,13 @@ class Project:
         :param int event_queue_maxsize: maxsize parameter to queue.Queue.
             0 means an unbounded queue.
         """
-        def post(desc):
-            exception = desc.get('_exception')
-            if exception:
-                raise exception
-            return self.construct_child(**desc)
-
         def create(root, name):
+            def post(desc):
+                exception = desc.get('_exception')
+                if exception:
+                    raise exception
+                return self.construct_child(name, **desc)
+
             with exception.add('Unable to create ' + name):
                 return recurse.recurse(
                     root,
@@ -43,10 +48,10 @@ class Project:
         attributes.check(kwds, 'project')
         self.path = path
         layout = layout or cleanup.cleanup_layout(animation)
-        self.maker = self.construct_child(**maker)
+        self.maker = self.construct_child('maker', **maker)
         self.drivers = [create(d, 'drivers') for d in drivers]
         with exception.add('Unable to create layout'):
-            self.layout = self.construct_child(**layout)
+            self.layout = self.construct_child('layout', **layout)
 
         self.animation = create(animation, 'animation')
 


Test patch:
diff --git a/test/bibliopixel/project/bad_project_test.py b/test/bibliopixel/project/bad_project_test.py
index b70e6a4..6cefa8b 100644
--- a/test/bibliopixel/project/bad_project_test.py
+++ b/test/bibliopixel/project/bad_project_test.py
@@ -6,10 +6,8 @@ from . make import make
 
 PYTHON_FILE = 'driver = {"a": "b"}'
 MISSING_LAYOUT = '{"animation": "off", "driver": "dummy"}'
-MISSING_ANIMATION = '{"layout": "matrix", "driver": "dummy"}'
 
 
-# This one works, but prints a warning.
 BAD_SECTION = """
 {
     "bad_section": {"foo": true},
@@ -19,13 +17,33 @@ BAD_SECTION = """
 }
 """
 
+BAD_SECTION2 = """
+{
+    "verbose": true,
+    "driver": "dummy",
+    "layout": "matrix",
+    "animation": "off"
+}
+"""
+
+MISSING_DATATYPE = """
+{
+    "driver": {
+         "width": 16,
+         "height": 16
+    },
+    "layout": "matrix",
+    "animation": "off"
+}
+"""
+
 BAD_DRIVER_ATTRIBUTE = """
 {
     "driver": {
          "typename": "simpixel",
          "width": 16,
          "height": 16,
-        "bad_attribute": 16
+         "bad_attribute": 16
     },
     "layout": "matrix",
     "animation": "off"
@@ -80,7 +98,24 @@ class BadProjectTest(unittest.TestCase):
 
         self.assertEquals(
             e.exception.args,
-            ('Unknown attribute for project: "bad_section"',))
+            ('There is no Project section named "bad_section"',))
+
+    def test_bad_section2(self):
+        # See https://github.com/ManiacalLabs/BiblioPixel/issues/787
+        with self.assertRaises(ValueError) as e:
+            make(BAD_SECTION2)
+
+        self.assertEquals(
+            e.exception.args,
+            ('There is no Project section named "verbose"',))
+
+    def test_missing_datatype(self):
+        with self.assertRaises(ValueError) as e:
+            make(MISSING_DATATYPE)
+        self.assertEquals(
+            e.exception.args,
+            ('Unable to create drivers',
+             'No "datatype" field in section "drivers"'))
 
     def test_bad_driver_attribute(self):
         with self.assertRaises(ValueError) as e:
@@ -117,16 +152,9 @@ class BadProjectTest(unittest.TestCase):
             e.exception.args,
             ('Missing "layout" section',))
 
-    def test_missing_animation(self):
-        with self.assertRaises(ValueError) as e:
-            make(MISSING_ANIMATION)
-        self.assertEquals(
-            e.exception.args,
-            ('Missing "animation" section',))
-
     def test_missing_everything(self):
         with self.assertRaises(ValueError) as e:
             make('{}')
         self.assertEquals(
             e.exception.args,
-            ('Missing "animation" section',))
+            ('Missing "layout" section',))
diff --git a/test/bibliopixel/project/cleanup_test.py b/test/bibliopixel/project/cleanup_test.py
index d529d10..9b3cdab 100644
--- a/test/bibliopixel/project/cleanup_test.py
+++ b/test/bibliopixel/project/cleanup_test.py
@@ -10,7 +10,6 @@ BASE = {
         'run': {},
         'datatype': OffAnim,
         'name': 'Off',
-        'typename': 'off',
     },
     'controls': [],
     'drivers': [{'typename': 'simpixel'}],
diff --git a/test/bibliopixel/project/recurse_test.py b/test/bibliopixel/project/recurse_test.py
index 8d0d992..cda5031 100644
--- a/test/bibliopixel/project/recurse_test.py
+++ b/test/bibliopixel/project/recurse_test.py
@@ -35,7 +35,6 @@ class RecurseTest(unittest.TestCase):
             'bing': 'bang',
         }
         expected = {
-            'typename': '%s.%s' % (Tester.__module__, Tester.__name__),
             'datatype': Tester,
             'foo': {'datatype': RecurseTest, 'foo': 'bar', 'color': colors.Red},
             'bing': 'post-bang',