From 7d30d417dc48ee60a515e9146816f3a279f025ac Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Mon, 3 Sep 2018 12:18:06 +0100
Subject: [PATCH 1/5] Convert test to use unittest framework.

---
 test.py | 163 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 103 insertions(+), 60 deletions(-)

diff --git a/test.py b/test.py
index 60ba0bf..4b7fb6a 100644
--- a/test.py
+++ b/test.py
@@ -1,60 +1,103 @@
-import coanacatl
-from shapely.geometry import Point
-from shapely.geometry import LineString
-from shapely.geometry import Polygon
-from shapely.geometry import MultiPoint
-from shapely.geometry import MultiLineString
-from shapely.geometry import MultiPolygon
-
-
-features = [
-    dict(
-        geometry=Point(0, 0),
-        properties={
-            'string': 'string_value',
-            'long': 4294967297L,
-            'int': 1,
-            'float': 1.0,
-            'bool': True,
-        },
-        id=1
-    ),
-    dict(
-        geometry=LineString([(0, 0), (1, 1)]),
-        properties={'baz': 'bat'},
-        id=None
-    ),
-    dict(
-        geometry=Point(0, 0).buffer(1),
-        properties={'blah': 'blah', 'id': 123},
-        id=3
-    ),
-    dict(
-        geometry=MultiPoint([(0, 0), (1, 1)]),
-        properties={'foo': 'bar', 'boolean': False},
-        id=None
-    ),
-    dict(
-        geometry=MultiLineString([[(0, 0), (1, 0)], [(0, 1), (1, 1)]]),
-        properties={'foo': 'bar'},
-        id=None
-    ),
-    dict(
-        geometry=Point(0, 0).buffer(0.4).union(Point(1, 1).buffer(0.4)),
-        properties={'blah': 'blah'},
-        id=4
-    ),
-]
-
-layers = [dict(
-    name='layer',
-    features=features,
-)]
-
-bounds = (0, 0, 1, 1)
-extents = 4096
-
-tile_data = coanacatl.encode(layers, bounds, extents)
-print repr(tile_data)
-with open('foo.mvt', 'w') as fh:
-    fh.write(tile_data)
+from unittest import TestCase
+
+
+class GeometryTest(TestCase):
+
+    def _generate_tile(self, features):
+        import coanacatl
+
+        layers = [dict(
+            name='layer',
+            features=features,
+        )]
+
+        bounds = (0, 0, 1, 1)
+        extents = 4096
+
+        tile_data = coanacatl.encode(layers, bounds, extents)
+        self.assertTrue(tile_data)
+        return tile_data
+
+    def test_point(self):
+        from shapely.geometry import Point
+
+        features = [
+            dict(
+                geometry=Point(0, 0),
+                properties={
+                    'string': 'string_value',
+                    'long': 4294967297L,
+                    'int': 1,
+                    'float': 1.0,
+                    'bool': True,
+                },
+                id=1
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_linestring(self):
+        from shapely.geometry import LineString
+
+        features = [
+            dict(
+                geometry=LineString([(0, 0), (1, 1)]),
+                properties={'baz': 'bat'},
+                id=None
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_polygon(self):
+        from shapely.geometry import Point
+
+        features = [
+            dict(
+                geometry=Point(0, 0).buffer(1),
+                properties={'blah': 'blah', 'id': 123},
+                id=3
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_multipoint(self):
+        from shapely.geometry import MultiPoint
+
+        features = [
+            dict(
+                geometry=MultiPoint([(0, 0), (1, 1)]),
+                properties={'foo': 'bar', 'boolean': False},
+                id=None
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_multilinestring(self):
+        from shapely.geometry import MultiLineString
+
+        features = [
+            dict(
+                geometry=MultiLineString([[(0, 0), (1, 0)], [(0, 1), (1, 1)]]),
+                properties={'foo': 'bar'},
+                id=None
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_multipolygon(self):
+        from shapely.geometry import Point
+
+        features = [
+            dict(
+                geometry=Point(0, 0).buffer(0.4).union(Point(1, 1).buffer(0.4)),
+                properties={'blah': 'blah'},
+                id=4
+            ),
+        ]
+
+        self._generate_tile(features)

From 5b869c2f0edb477546eb4f3751f3911b37a3375d Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Mon, 3 Sep 2018 12:33:35 +0100
Subject: [PATCH 2/5] Fix for unicode layer names, property keys. Add tests for
 them.

---
 coanacatl/coanacatl.cpp |  43 +++++++++++----
 test.py                 | 112 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 133 insertions(+), 22 deletions(-)

diff --git a/coanacatl/coanacatl.cpp b/coanacatl/coanacatl.cpp
index de71919..57c434d 100644
--- a/coanacatl/coanacatl.cpp
+++ b/coanacatl/coanacatl.cpp
@@ -41,6 +41,36 @@ void _coanacatl_printf(const char *fmt, ...) {
 #define FINISH_GEOS finishGEOS_r
 #endif
 
+namespace {
+
+/**
+ * Extract a string from a Python object. The object _must_ be either a str or
+ * unicode object, else an exception will be thrown.
+ */
+std::string extract_utf8_string(bp::object value) {
+  PyObject *value_ptr = value.ptr();
+
+  if (PyUnicode_Check(value_ptr)) {
+    bp::object encoded = bp::str(value).encode("utf-8");
+    std::string v = bp::extract<std::string>(encoded);
+    return v;
+
+  } else if (PyString_Check(value_ptr)) {
+    std::string v = bp::extract<std::string>(value);
+    return v;
+
+  } else {
+    std::ostringstream out;
+    bp::object repr_py = value.attr("__repr__")();
+    std::string repr = bp::extract<std::string>(repr_py);
+    out << "Unable to convert Python object of type "
+        << value_ptr->ob_type->tp_name << " to string: " << repr;
+    throw std::runtime_error(out.str());
+  }
+}
+
+} // end anonymous namespace
+
 class encoder {
 public:
   encoder(bp::tuple bounds, size_t extents)
@@ -121,7 +151,7 @@ class encoder {
 };
 
 void encoder::encode_layer(bp::object layer) {
-  std::string layer_name = bp::extract<std::string>(layer["name"]);
+  std::string layer_name = extract_utf8_string(layer["name"]);
 
   if (m_layer_names.count(layer_name) > 0) {
     throw std::runtime_error("Duplicate layer names are not allowed.");
@@ -158,7 +188,7 @@ void encoder::add_properties(vtzero::feature_builder &fb, bp::dict props) {
   const size_t num_items = bp::len(items);
   for (size_t i = 0; i < num_items; ++i) {
     bp::object item = items[i];
-    std::string k = bp::extract<std::string>(item[0]);
+    std::string k = extract_utf8_string(item[0]);
     bp::object value = item[1];
     PyObject *value_ptr = value.ptr();
 
@@ -174,13 +204,8 @@ void encoder::add_properties(vtzero::feature_builder &fb, bp::dict props) {
       int64_t v = bp::extract<int64_t>(value);
       fb.add_property(k, v);
 
-    } else if (PyUnicode_Check(value_ptr)) {
-      bp::object encoded = bp::str(value).encode("utf-8");
-      std::string v = bp::extract<std::string>(encoded);
-      fb.add_property(k, v);
-
-    } else if (PyString_Check(value_ptr)) {
-      std::string v = bp::extract<std::string>(value);
+    } else if (PyUnicode_Check(value_ptr) || PyString_Check(value_ptr)) {
+      std::string v = extract_utf8_string(value);
       fb.add_property(k, v);
 
     } else {
diff --git a/test.py b/test.py
index 4b7fb6a..ef8d226 100644
--- a/test.py
+++ b/test.py
@@ -24,13 +24,7 @@ def test_point(self):
         features = [
             dict(
                 geometry=Point(0, 0),
-                properties={
-                    'string': 'string_value',
-                    'long': 4294967297L,
-                    'int': 1,
-                    'float': 1.0,
-                    'bool': True,
-                },
+                properties={},
                 id=1
             ),
         ]
@@ -43,7 +37,7 @@ def test_linestring(self):
         features = [
             dict(
                 geometry=LineString([(0, 0), (1, 1)]),
-                properties={'baz': 'bat'},
+                properties={},
                 id=None
             ),
         ]
@@ -56,7 +50,7 @@ def test_polygon(self):
         features = [
             dict(
                 geometry=Point(0, 0).buffer(1),
-                properties={'blah': 'blah', 'id': 123},
+                properties={},
                 id=3
             ),
         ]
@@ -69,7 +63,7 @@ def test_multipoint(self):
         features = [
             dict(
                 geometry=MultiPoint([(0, 0), (1, 1)]),
-                properties={'foo': 'bar', 'boolean': False},
+                properties={},
                 id=None
             ),
         ]
@@ -82,7 +76,7 @@ def test_multilinestring(self):
         features = [
             dict(
                 geometry=MultiLineString([[(0, 0), (1, 0)], [(0, 1), (1, 1)]]),
-                properties={'foo': 'bar'},
+                properties={},
                 id=None
             ),
         ]
@@ -94,10 +88,102 @@ def test_multipolygon(self):
 
         features = [
             dict(
-                geometry=Point(0, 0).buffer(0.4).union(Point(1, 1).buffer(0.4)),
-                properties={'blah': 'blah'},
+                geometry=Point(0, 0).buffer(0.4).union(
+                    Point(1, 1).buffer(0.4)),
+                properties={},
                 id=4
             ),
         ]
 
         self._generate_tile(features)
+
+
+class PropertyTest(TestCase):
+
+    def _generate_tile(self, features):
+        import coanacatl
+
+        layers = [dict(
+            name='layer',
+            features=features,
+        )]
+
+        bounds = (0, 0, 1, 1)
+        extents = 4096
+
+        tile_data = coanacatl.encode(layers, bounds, extents)
+        self.assertTrue(tile_data)
+        return tile_data
+
+    def test_property_types(self):
+        from shapely.geometry import Point
+
+        features = [
+            dict(
+                geometry=Point(0, 0),
+                properties={
+                    'string': 'string_value',
+                    'long': 4294967297L,
+                    'int': 1,
+                    'float': 1.0,
+                    'bool': True,
+                },
+                id=1
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_unicode_property_value(self):
+        from shapely.geometry import Point
+
+        features = [
+            dict(
+                geometry=Point(0, 0),
+                properties={
+                    'string': unicode('unicode_value'),
+                },
+                id=1
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_unicode_property_key(self):
+        from shapely.geometry import Point
+
+        features = [
+            dict(
+                geometry=Point(0, 0),
+                properties={
+                    unicode('unicode'): 'string_value',
+                },
+                id=1
+            ),
+        ]
+
+        self._generate_tile(features)
+
+    def test_unicode_layer_name(self):
+        import coanacatl
+        from shapely.geometry import Point
+
+        layers = [dict(
+            name=unicode('layer'),
+            features=[
+                dict(
+                    geometry=Point(0, 0),
+                    properties={
+                        'foo': 'bar',
+                    },
+                    id=1
+                ),
+            ],
+        )]
+
+        bounds = (0, 0, 1, 1)
+        extents = 4096
+
+        tile_data = coanacatl.encode(layers, bounds, extents)
+        self.assertTrue(tile_data)
+        return tile_data

From e74e5a34d315dc2c2ba0239c92c37302ada9566a Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Mon, 3 Sep 2018 12:41:04 +0100
Subject: [PATCH 3/5] Add basic CircleCI configuration.

---
 .circleci/config.yml | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 .circleci/config.yml

diff --git a/.circleci/config.yml b/.circleci/config.yml
new file mode 100644
index 0000000..3bb33f2
--- /dev/null
+++ b/.circleci/config.yml
@@ -0,0 +1,19 @@
+version: 2
+jobs:
+  build:
+    docker:
+      - image: circleci/python:2.7.15-stretch
+    steps:
+      - checkout
+      - run:
+          name: Install C++ dependencies
+          command: sudo apt install build-essential libgeos-dev libboost-python-dev
+      - run:
+          name: Install Python dependencies
+          command: sudo pip install shapely
+      - run:
+          name: Build library
+          command: python setup.py build
+      - run:
+          name: Unit tests
+          command: python setup.py test

From 41accd734d34a573f8fe6f70ed66c2e646960f48 Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Mon, 3 Sep 2018 12:49:36 +0100
Subject: [PATCH 4/5] Add missing dependencies to the README. Update a few
 comments to reflect progress.

---
 README.md | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index e4b4286..e11dd59 100644
--- a/README.md
+++ b/README.md
@@ -21,7 +21,13 @@ Where:
 You will need to install a C++11 build system and the GEOS library, e.g: if you are on Ubuntu or Debian:
 
 ```
-sudo apt install build-essential libgeos-dev
+sudo apt install build-essential libgeos-dev libboost-python-dev
+```
+
+You will also need the [Shapely](http://toblerity.org/shapely/) Python library. Install (with or without `sudo` depending on whether you're installing it globally or locally):
+
+```
+pip install shapely
 ```
 
 **NOTE: probably other stuff as well! Please [file an issue](https://github.com/tilezen/coanacatl/issues/new) if you find you need additional dependencies.**
@@ -41,8 +47,8 @@ python setup.py install
 ## Current limitations
 
 * Only point, linestring, polygon and multi-versions of those are supported. Linear rings and geometry collections are currently not supported.
-* Property dictionary keys must be strings, as per the MVT spec. Property dictionary values can be boolean, integer, floating point or strings.
-* There are **no tests**!
+* Property dictionary keys must be strings (or `unicode`), as per the MVT spec. Property dictionary values can be boolean, integer, floating point or strings.
+* There are **very few tests**!
 * Error checking of return values from the GEOS API is inadequate, and needs shoring up.
 * There needs to be a better way to return warnings/errors to the user, perhaps as a list of objects, so that the user can determine if it's enough to fail the tile or just log.
 

From 2d51cda78603ceb312d747f5e57b788147aabb22 Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Mon, 3 Sep 2018 12:51:25 +0100
Subject: [PATCH 5/5] Checkout submodules in CI.

---
 .circleci/config.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 3bb33f2..1afae36 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -5,6 +5,9 @@ jobs:
       - image: circleci/python:2.7.15-stretch
     steps:
       - checkout
+      - run:
+          name: Checkout submodules
+          command: git submodule update --init --recursive
       - run:
           name: Install C++ dependencies
           command: sudo apt install build-essential libgeos-dev libboost-python-dev