Raise ValueError on setting bad Overlay / GroundOverlay members instead of

coercing to a valid type or defaulting to None.

fastkml/kml.py

@@ -686,7 +686,12 @@ class _Overlay(_Feature):
 
     @color.setter
     def color(self, color):
-        self._color = color
+        if isinstance(color, basestring):
+            self._color = color
+        elif color is None:
+            self._color = None
+        else:
+            raise ValueError
 
     @property
     def drawOrder(self):
@@ -694,12 +699,12 @@ class _Overlay(_Feature):
 
     @drawOrder.setter
     def drawOrder(self, value):
-        try:
-            int(value)
-        except TypeError:
+        if isinstance(value, (basestring, int, float)):
+            self._drawOrder = str(value)
+        elif value is None:
             self._drawOrder = None
         else:
-            self._drawOrder = str(value)
+            raise ValueError
 
     @property
     def icon(self):
@@ -811,12 +816,12 @@ class GroundOverlay(_Overlay):
 
     @altitude.setter
     def altitude(self, value):
-        try:
-            float(value)
-        except (ValueError, TypeError):
+        if isinstance(value, (basestring, int, float)):
+            self._altitude = str(value)
+        elif value is None:
             self._altitude = None
         else:
-            self._altitude = str(value)
+            raise ValueError
 
     @property
     def altitudeMode(self):
@@ -835,12 +840,12 @@ class GroundOverlay(_Overlay):
 
     @north.setter
     def north(self, value):
-        try:
-            float(value)
-        except (ValueError, TypeError):
+        if isinstance(value, (basestring, int, float)):
+            self._north = str(value)
+        elif value is None:
             self._north = None
         else:
-            self._north = str(value)
+            raise ValueError
 
     @property
     def south(self):
@@ -848,12 +853,12 @@ class GroundOverlay(_Overlay):
 
     @south.setter
     def south(self, value):
-        try:
-            float(value)
-        except (ValueError, TypeError):
+        if isinstance(value, (basestring, int, float)):
+            self._south = str(value)
+        elif value is None:
             self._south = None
         else:
-            self._south = str(value)
+            raise ValueError
 
     @property
     def east(self):
@@ -861,12 +866,12 @@ class GroundOverlay(_Overlay):
 
     @east.setter
     def east(self, value):
-        try:
-            float(value)
-        except (ValueError, TypeError):
+        if isinstance(value, (basestring, int, float)):
+            self._east = str(value)
+        elif value is None:
             self._east = None
         else:
-            self._east = str(value)
+            raise ValueError
 
     @property
     def west(self):
@@ -874,12 +879,12 @@ class GroundOverlay(_Overlay):
 
     @west.setter
     def west(self, value):
-        try:
-            float(value)
-        except (ValueError, TypeError):
+        if isinstance(value, (basestring, int, float)):
+            self._west = str(value)
+        elif value is None:
             self._west = None
         else:
-            self._west = str(value)
+            raise ValueError
 
     @property
     def rotation(self):
@@ -887,29 +892,20 @@ class GroundOverlay(_Overlay):
 
     @rotation.setter
     def rotation(self, value):
-        try:
-            float(value)
-        except (ValueError, TypeError):
+        if isinstance(value, (basestring, int, float)):
+            self._rotation = str(value)
+        elif value is None:
             self._rotation = None
         else:
-            self._rotation = str(value)
+            raise ValueError
 
     def latLonBox(self, north, south, east, west, rotation=0):
         # TODO: Check for bounds (0:+/-90 or 0:+/-180 degrees)
-        try:
-            float(north)
-            float(south)
-            float(east)
-            float(west)
-            float(rotation)
-        except:
-            raise ValueError
-        else:
-            self._north = str(north)
-            self._south = str(south)
-            self._east = str(east)
-            self._west = str(west)
-            self._rotation = str(rotation)
+        self.north = north
+        self.south = south
+        self.east = east
+        self.west = west
+        self.rotation = rotation
 
     def etree_element(self):
         element = super(GroundOverlay, self).etree_element()

fastkml/test_main.py

@@ -2020,11 +2020,23 @@ class BaseFeatureTestCase(unittest.TestCase):
 
 
 class BaseOverlayTestCase(unittest.TestCase):
-    def test_color(self):
+    def test_color_string(self):
         o = kml._Overlay(name='An Overlay')
         o.color = '00010203'
         self.assertEqual(o.color, '00010203')
 
+    def test_color_none(self):
+        o = kml._Overlay(name='An Overlay')
+        o.color = '00010203'
+        self.assertEqual(o.color, '00010203')
+        o.color = None
+        self.assertEqual(o.color, None)
+
+    def test_color_value_error(self):
+        o = kml._Overlay(name='An Overlay')
+        with self.assertRaises(ValueError):
+            o.color = object()
+
     def test_draw_order_string(self):
         o = kml._Overlay(name='An Overlay')
         o.drawOrder = '1'
@@ -2035,11 +2047,18 @@ class BaseOverlayTestCase(unittest.TestCase):
         o.drawOrder = 1
         self.assertEqual(o.drawOrder, '1')
 
-    def test_draw_order_value_error(self):
+    def test_draw_order_none(self):
         o = kml._Overlay(name='An Overlay')
-        o.drawOrder = object()
+        o.drawOrder = '1'
+        self.assertEqual(o.drawOrder, '1')
+        o.drawOrder = None
         self.assertEqual(o.drawOrder, None)
 
+    def test_draw_order_value_error(self):
+        o = kml._Overlay(name='An Overlay')
+        with self.assertRaises(ValueError):
+            o.drawOrder = object()
+
     def test_icon_without_tag(self):
         o = kml._Overlay(name='An Overlay')
         o.icon = 'http://example.com/'
@@ -2089,12 +2108,14 @@ class GroundOverlayTestCase(unittest.TestCase):
         self.g.altitude = '123'
         self.assertEqual(self.g.altitude, '123')
 
-    def test_altitude_type_error(self):
-        self.g.altitude = object()
-        self.assertEqual(self.g.altitude, None)
-
     def test_altitude_value_error(self):
-        self.g.altitude = ''
+        with self.assertRaises(ValueError):
+            self.g.altitude = object()
+
+    def test_altitude_none(self):
+        self.g.altitude = '123'
+        self.assertEqual(self.g.altitude, '123')
+        self.g.altitude = None
         self.assertEqual(self.g.altitude, None)
 
     def test_altitude_mode_default(self):
@@ -2112,11 +2133,29 @@ class GroundOverlayTestCase(unittest.TestCase):
         self.g.altitudeMode = 'absolute'
         self.assertEqual(self.g.altitudeMode, 'absolute')
 
-    def test_latlonbox_function_value_error(self):
-        with self.assertRaises(ValueError):
-            self.g.latLonBox('', '', '', '', '')
+    def test_latlonbox_function(self):
+        self.g.latLonBox(10, 20, 30, 40, 50)
+
+        self.assertEqual(self.g.north, '10')
+        self.assertEqual(self.g.south, '20')
+        self.assertEqual(self.g.east, '30')
+        self.assertEqual(self.g.west, '40')
+        self.assertEqual(self.g.rotation, '50')
+
+    def test_latlonbox_string(self):
+        self.g.north = '10'
+        self.g.south = '20'
+        self.g.east = '30'
+        self.g.west = '40'
+        self.g.rotation = '50'
+
+        self.assertEqual(self.g.north, '10')
+        self.assertEqual(self.g.south, '20')
+        self.assertEqual(self.g.east, '30')
+        self.assertEqual(self.g.west, '40')
+        self.assertEqual(self.g.rotation, '50')
 
-    def test_latlonbox_properties(self):
+    def test_latlonbox_int(self):
         self.g.north = 10
         self.g.south = 20
         self.g.east = 30
@@ -2129,12 +2168,34 @@ class GroundOverlayTestCase(unittest.TestCase):
         self.assertEqual(self.g.west, '40')
         self.assertEqual(self.g.rotation, '50')
 
-    def test_latlonbox_properties_type_error(self):
-        self.g.north = object()
-        self.g.south = object()
-        self.g.east = object()
-        self.g.west = object()
-        self.g.rotation = object()
+    def test_latlonbox_float(self):
+        self.g.north = 10.0
+        self.g.south = 20.0
+        self.g.east = 30.0
+        self.g.west = 40.0
+        self.g.rotation = 50.0
+
+        self.assertEqual(self.g.north, '10.0')
+        self.assertEqual(self.g.south, '20.0')
+        self.assertEqual(self.g.east, '30.0')
+        self.assertEqual(self.g.west, '40.0')
+        self.assertEqual(self.g.rotation, '50.0')
+
+    def test_latlonbox_value_error(self):
+        with self.assertRaises(ValueError):
+            self.g.north = object()
+
+        with self.assertRaises(ValueError):
+            self.g.south = object()
+
+        with self.assertRaises(ValueError):
+            self.g.east = object()
+
+        with self.assertRaises(ValueError):
+            self.g.west = object()
+
+        with self.assertRaises(ValueError):
+            self.g.rotation = object()
 
         self.assertEqual(self.g.north, None)
         self.assertEqual(self.g.south, None)
@@ -2142,13 +2203,26 @@ class GroundOverlayTestCase(unittest.TestCase):
         self.assertEqual(self.g.west, None)
         self.assertEqual(self.g.rotation, None)
 
-    def test_latlonbox_properties_value_error(self):
+    def test_latlonbox_empty_string(self):
         self.g.north = ''
         self.g.south = ''
         self.g.east = ''
         self.g.west = ''
         self.g.rotation = ''
 
+        self.assertEqual(self.g.north, '')
+        self.assertEqual(self.g.south, '')
+        self.assertEqual(self.g.east, '')
+        self.assertEqual(self.g.west, '')
+        self.assertEqual(self.g.rotation, '')
+
+    def test_latlonbox_none(self):
+        self.g.north = None
+        self.g.south = None
+        self.g.east = None
+        self.g.west = None
+        self.g.rotation = None
+
         self.assertEqual(self.g.north, None)
         self.assertEqual(self.g.south, None)
         self.assertEqual(self.g.east, None)