Merge pull request #469 from CartoDB/development

Release 0.17.0

NEWS.md

@@ -1,3 +1,15 @@
+February 22th, 2018
+==================
+* Version `0.17.0` of the python library
+    * Change default provider to Mapbox
+    * Remove the obligatory nature of the Mapzen configuration due to its deprecation as provider
+
+February 13th, 2018
+==================
+* Version `0.16.7` of the python library
+    * Pick the first result when Mapbox geocoder returns multiple results #462
+    * Normalize input for Mapbox geocoder #462
+
 February 12th, 2018
 ==================
 * Version `0.16.6` of the python library

client/test/expected/00_installation_test.out

@@ -1,3 +1,5 @@
+-- Only show warning or error messages in the tests output
+SET client_min_messages TO WARNING;
 -- Install dependencies
 CREATE EXTENSION postgis;
 CREATE EXTENSION plpythonu;

client/test/sql/00_installation_test.sql

@@ -1,3 +1,5 @@
+-- Only show warning or error messages in the tests output
+SET client_min_messages TO WARNING;
 -- Install dependencies
 CREATE EXTENSION postgis;
 CREATE EXTENSION plpythonu;

server/extension/test/expected/00_install_test.out

@@ -1,3 +1,5 @@
+-- Only show warning or error messages in the tests output
+SET client_min_messages TO WARNING;
 -- Install dependencies
 CREATE EXTENSION postgis;
 CREATE EXTENSION plpythonu;

server/extension/test/sql/00_install_test.sql

@@ -1,3 +1,5 @@
+-- Only show warning or error messages in the tests output
+SET client_min_messages TO WARNING;
 -- Install dependencies
 CREATE EXTENSION postgis;
 CREATE EXTENSION plpythonu;

server/lib/python/cartodb_services/README.md

@@ -24,7 +24,7 @@ NOTE: a system installation is required at present because the library is meant
 
 
 ## Running the unit tests
-Just run `nosetests test/`
+Just run `MAPBOX_API_KEY=xxx nosetests test/`
 ```shell
 $ nosetests test/
 ......................................................................................................

server/lib/python/cartodb_services/cartodb_services/mapbox/geocoder.py

@@ -8,6 +8,7 @@ from mapbox import Geocoder
 from cartodb_services.metrics import Traceable
 from cartodb_services.tools.exceptions import ServiceException
 from cartodb_services.tools.qps import qps_retry
+from cartodb_services.tools.normalize import normalize
 
 GEOCODER_NAME = 'geocoder_name'
 EPHEMERAL_GEOCODER = 'mapbox.places'
@@ -39,10 +40,16 @@ class MapboxGeocoder(Traceable):
     def _parse_geocoder_response(self, response):
         json_response = json.loads(response)
 
-        if json_response and json_response[ENTRY_FEATURES]:
-            feature = json_response[ENTRY_FEATURES][0]
+        # If Mapbox returns more that one result, take the first one
+        if json_response:
+            if type(json_response) == list:
+                json_response = json_response[0]
 
-            return self._extract_lng_lat_from_feature(feature)
+            if json_response[ENTRY_FEATURES]:
+                feature = json_response[ENTRY_FEATURES][0]
+                return self._extract_lng_lat_from_feature(feature)
+            else:
+                return []
         else:
             return []
 
@@ -61,11 +68,11 @@ class MapboxGeocoder(Traceable):
     def geocode(self, searchtext, city=None, state_province=None,
                 country=None):
         if searchtext and searchtext.strip():
-            address = [searchtext]
+            address = [normalize(searchtext)]
             if city:
-                address.append(city)
+                address.append(normalize(city))
             if state_province:
-                address.append(state_province)
+                address.append(normalize(state_province))
         else:
             return []
 

server/lib/python/cartodb_services/cartodb_services/metrics/config.py

@@ -136,7 +136,7 @@ class RoutingConfig(ServiceConfig):
     ROUTING_PROVIDER_KEY = 'routing_provider'
     MAPZEN_PROVIDER = 'mapzen'
     MAPBOX_PROVIDER = 'mapbox'
-    DEFAULT_PROVIDER = MAPZEN_PROVIDER
+    DEFAULT_PROVIDER = MAPBOX_PROVIDER
     QUOTA_KEY = 'mapzen_routing_quota'
     SOFT_LIMIT_KEY = 'soft_mapzen_routing_limit'
     METRICS_LOG_KEY = 'routing_log_path'
@@ -226,7 +226,7 @@ class IsolinesRoutingConfig(ServiceConfig):
     MAPZEN_PROVIDER = 'mapzen'
     MAPBOX_PROVIDER = 'mapbox'
     HEREMAPS_PROVIDER = 'heremaps'
-    DEFAULT_PROVIDER = MAPZEN_PROVIDER
+    DEFAULT_PROVIDER = MAPBOX_PROVIDER
     METRICS_LOG_KEY = 'isolines_log_path'
 
     def __init__(self, redis_connection, db_conn, username, orgname=None):
@@ -391,7 +391,7 @@ class GeocoderConfig(ServiceConfig):
     USERNAME_KEY = 'username'
     ORGNAME_KEY = 'orgname'
     PERIOD_END_DATE = 'period_end_date'
-    DEFAULT_PROVIDER = MAPZEN_GEOCODER
+    DEFAULT_PROVIDER = MAPBOX_GEOCODER
     METRICS_LOG_KEY = 'geocoder_log_path'
 
     def __init__(self, redis_connection, db_conn, username, orgname=None, forced_provider=None):
@@ -576,50 +576,51 @@ class ServicesDBConfig:
         heremaps_conf_json = self._get_conf('heremaps_conf')
         if not heremaps_conf_json:
             raise ConfigException('Here maps configuration missing')
-        else:
-            heremaps_conf = json.loads(heremaps_conf_json)
-            self._heremaps_geocoder_app_id = heremaps_conf['geocoder']['app_id']
-            self._heremaps_geocoder_app_code = heremaps_conf['geocoder']['app_code']
-            self._heremaps_geocoder_cost_per_hit = heremaps_conf['geocoder'][
-                'geocoder_cost_per_hit']
-            self._heremaps_geocoder_service_params = heremaps_conf['geocoder'].get('service', {})
-            self._heremaps_isolines_app_id = heremaps_conf['isolines']['app_id']
-            self._heremaps_isolines_app_code = heremaps_conf['isolines']['app_code']
-            self._heremaps_isolines_service_params = heremaps_conf['isolines'].get('service', {})
+
+        heremaps_conf = json.loads(heremaps_conf_json)
+        self._heremaps_geocoder_app_id = heremaps_conf['geocoder']['app_id']
+        self._heremaps_geocoder_app_code = heremaps_conf['geocoder']['app_code']
+        self._heremaps_geocoder_cost_per_hit = heremaps_conf['geocoder'][
+            'geocoder_cost_per_hit']
+        self._heremaps_geocoder_service_params = heremaps_conf['geocoder'].get('service', {})
+        self._heremaps_isolines_app_id = heremaps_conf['isolines']['app_id']
+        self._heremaps_isolines_app_code = heremaps_conf['isolines']['app_code']
+        self._heremaps_isolines_service_params = heremaps_conf['isolines'].get('service', {})
 
     def _get_mapzen_config(self):
         mapzen_conf_json = self._get_conf('mapzen_conf')
+        # We dont use mapzen anymore so we don't need to check for its configuration
         if not mapzen_conf_json:
-            raise ConfigException('Mapzen configuration missing')
-        else:
-            mapzen_conf = json.loads(mapzen_conf_json)
-            self._mapzen_matrix_api_key = mapzen_conf['matrix']['api_key']
-            self._mapzen_matrix_quota = mapzen_conf['matrix']['monthly_quota']
-            self._mapzen_matrix_service_params = mapzen_conf['matrix'].get('service', {})
-            self._mapzen_isochrones_service_params = mapzen_conf.get('isochrones', {}).get('service', {})
-            self._mapzen_routing_api_key = mapzen_conf['routing']['api_key']
-            self._mapzen_routing_quota = mapzen_conf['routing']['monthly_quota']
-            self._mapzen_routing_service_params = mapzen_conf['routing'].get('service', {})
-            self._mapzen_geocoder_api_key = mapzen_conf['geocoder']['api_key']
-            self._mapzen_geocoder_quota = mapzen_conf['geocoder']['monthly_quota']
-            self._mapzen_geocoder_service_params = mapzen_conf['geocoder'].get('service', {})
+            return
+
+        mapzen_conf = json.loads(mapzen_conf_json)
+        self._mapzen_matrix_api_key = mapzen_conf['matrix']['api_key']
+        self._mapzen_matrix_quota = mapzen_conf['matrix']['monthly_quota']
+        self._mapzen_matrix_service_params = mapzen_conf['matrix'].get('service', {})
+        self._mapzen_isochrones_service_params = mapzen_conf.get('isochrones', {}).get('service', {})
+        self._mapzen_routing_api_key = mapzen_conf['routing']['api_key']
+        self._mapzen_routing_quota = mapzen_conf['routing']['monthly_quota']
+        self._mapzen_routing_service_params = mapzen_conf['routing'].get('service', {})
+        self._mapzen_geocoder_api_key = mapzen_conf['geocoder']['api_key']
+        self._mapzen_geocoder_quota = mapzen_conf['geocoder']['monthly_quota']
+        self._mapzen_geocoder_service_params = mapzen_conf['geocoder'].get('service', {})
 
     def _get_mapbox_config(self):
         mapbox_conf_json = self._get_conf('mapbox_conf')
         if not mapbox_conf_json:
             raise ConfigException('Mapbox configuration missing')
-        else:
-            mapbox_conf = json.loads(mapbox_conf_json)
-            self._mapbox_matrix_api_keys = mapbox_conf['matrix']['api_keys']
-            self._mapbox_matrix_quota = mapbox_conf['matrix']['monthly_quota']
-            self._mapbox_matrix_service_params = mapbox_conf['matrix'].get('service', {})
-            self._mapbox_isochrones_service_params = mapbox_conf.get('isochrones', {}).get('service', {})
-            self._mapbox_routing_api_keys = mapbox_conf['routing']['api_keys']
-            self._mapbox_routing_quota = mapbox_conf['routing']['monthly_quota']
-            self._mapbox_routing_service_params = mapbox_conf['routing'].get('service', {})
-            self._mapbox_geocoder_api_keys = mapbox_conf['geocoder']['api_keys']
-            self._mapbox_geocoder_quota = mapbox_conf['geocoder']['monthly_quota']
-            self._mapbox_geocoder_service_params = mapbox_conf['geocoder'].get('service', {})
+
+        mapbox_conf = json.loads(mapbox_conf_json)
+        self._mapbox_matrix_api_keys = mapbox_conf['matrix']['api_keys']
+        self._mapbox_matrix_quota = mapbox_conf['matrix']['monthly_quota']
+        self._mapbox_matrix_service_params = mapbox_conf['matrix'].get('service', {})
+        self._mapbox_isochrones_service_params = mapbox_conf.get('isochrones', {}).get('service', {})
+        self._mapbox_routing_api_keys = mapbox_conf['routing']['api_keys']
+        self._mapbox_routing_quota = mapbox_conf['routing']['monthly_quota']
+        self._mapbox_routing_service_params = mapbox_conf['routing'].get('service', {})
+        self._mapbox_geocoder_api_keys = mapbox_conf['geocoder']['api_keys']
+        self._mapbox_geocoder_quota = mapbox_conf['geocoder']['monthly_quota']
+        self._mapbox_geocoder_service_params = mapbox_conf['geocoder'].get('service', {})
 
     def _get_data_observatory_config(self):
         do_conf_json = self._get_conf('data_observatory_conf')

server/lib/python/cartodb_services/cartodb_services/tools/normalize.py

@@ -0,0 +1,3 @@
+def normalize(str_input):
+    return str_input.replace('"', '"') \
+                    .replace(';', ',')

server/lib/python/cartodb_services/setup.py

@@ -10,7 +10,7 @@ from setuptools import setup, find_packages
 setup(
     name='cartodb_services',
 
-    version='0.16.6',
+    version='0.17.0',
 
     description='CartoDB Services API Python Library',
 

server/lib/python/cartodb_services/test/credentials.py

@@ -0,0 +1,6 @@
+import os
+
+def mapbox_api_key():
+    """Returns Mapbox API key. Requires setting MAPBOX_API_KEY environment variable."""
+    return os.environ['MAPBOX_API_KEY']
+

server/lib/python/cartodb_services/test/test_mapboxgeocoder.py

@@ -2,8 +2,8 @@ import unittest
 from mock import Mock
 from cartodb_services.mapbox import MapboxGeocoder
 from cartodb_services.tools.exceptions import ServiceException
+from credentials import mapbox_api_key
 
-VALID_TOKEN = 'pk.eyJ1IjoiYWNhcmxvbiIsImEiOiJjamJuZjQ1Zjc0Ymt4Mnh0YmFrMmhtYnY4In0.gt9cw0VeKc3rM2mV5pcEmg'
 INVALID_TOKEN = 'invalid_token'
 VALID_ADDRESS = 'Calle Siempreviva 3, Valladolid'
 WELL_KNOWN_LONGITUDE = -4.730947
@@ -12,7 +12,7 @@ WELL_KNOWN_LATITUDE = 41.668654
 
 class MapboxGeocoderTestCase(unittest.TestCase):
     def setUp(self):
-        self.geocoder = MapboxGeocoder(token=VALID_TOKEN, logger=Mock())
+        self.geocoder = MapboxGeocoder(token=mapbox_api_key(), logger=Mock())
 
     def test_invalid_token(self):
         invalid_geocoder = MapboxGeocoder(token=INVALID_TOKEN, logger=Mock())
@@ -35,6 +35,11 @@ class MapboxGeocoderTestCase(unittest.TestCase):
 
         assert place
 
+    def test_odd_characters(self):
+        place = self.geocoder.geocode(searchtext='Barcelona; "Spain"')
+
+        assert place
+
     def test_empty_request(self):
         place = self.geocoder.geocode(searchtext='', country=None, city=None, state_province=None)
 

server/lib/python/cartodb_services/test/test_mapboxisoline.py

@@ -7,15 +7,15 @@ from cartodb_services.mapbox.routing import MapboxRouting
 from cartodb_services.tools import Coordinate
 from cartodb_services.tools.coordinates import (validate_coordinates,
                                                 marshall_coordinates)
+from credentials import mapbox_api_key
 
-VALID_TOKEN = 'pk.eyJ1IjoiYWNhcmxvbiIsImEiOiJjamJuZjQ1Zjc0Ymt4Mnh0YmFrMmhtYnY4In0.gt9cw0VeKc3rM2mV5pcEmg'
 VALID_ORIGIN = Coordinate(-73.989, 40.733)
 
 
 class MapboxIsolinesTestCase(unittest.TestCase):
 
     def setUp(self):
-        matrix_client = MapboxMatrixClient(token=VALID_TOKEN, logger=Mock())
+        matrix_client = MapboxMatrixClient(token=mapbox_api_key(), logger=Mock())
         self.mapbox_isolines = MapboxIsolines(matrix_client, logger=Mock())
 
     def test_calculate_isochrone(self):

server/lib/python/cartodb_services/test/test_mapboxmatrix.py

@@ -4,8 +4,8 @@ from cartodb_services.mapbox import MapboxMatrixClient
 from cartodb_services.mapbox.matrix_client import DEFAULT_PROFILE
 from cartodb_services.tools.exceptions import ServiceException
 from cartodb_services.tools import Coordinate
+from credentials import mapbox_api_key
 
-VALID_TOKEN = 'pk.eyJ1IjoiYWNhcmxvbiIsImEiOiJjamJuZjQ1Zjc0Ymt4Mnh0YmFrMmhtYnY4In0.gt9cw0VeKc3rM2mV5pcEmg'
 INVALID_TOKEN = 'invalid_token'
 VALID_ORIGIN = Coordinate(-73.989, 40.733)
 VALID_TARGET = Coordinate(-74, 40.733)
@@ -22,7 +22,7 @@ INVALID_PROFILE = 'invalid_profile'
 
 class MapboxMatrixTestCase(unittest.TestCase):
     def setUp(self):
-        self.matrix_client = MapboxMatrixClient(token=VALID_TOKEN,
+        self.matrix_client = MapboxMatrixClient(token=mapbox_api_key(),
                                                 logger=Mock())
 
     def test_invalid_profile(self):

server/lib/python/cartodb_services/test/test_mapboxrouting.py

@@ -4,8 +4,8 @@ from cartodb_services.mapbox import MapboxRouting
 from cartodb_services.mapbox.routing import DEFAULT_PROFILE
 from cartodb_services.tools.exceptions import ServiceException
 from cartodb_services.tools import Coordinate
+from credentials import mapbox_api_key
 
-VALID_TOKEN = 'pk.eyJ1IjoiYWNhcmxvbiIsImEiOiJjamJuZjQ1Zjc0Ymt4Mnh0YmFrMmhtYnY4In0.gt9cw0VeKc3rM2mV5pcEmg'
 INVALID_TOKEN = 'invalid_token'
 VALID_WAYPOINTS = [Coordinate(-73.989, 40.733), Coordinate(-74, 40.733)]
 NUM_WAYPOINTS_MAX = 25
@@ -31,7 +31,7 @@ WELL_KNOWN_LENGTH = 1317.9
 
 class MapboxRoutingTestCase(unittest.TestCase):
     def setUp(self):
-        self.routing = MapboxRouting(token=VALID_TOKEN, logger=Mock())
+        self.routing = MapboxRouting(token=mapbox_api_key(), logger=Mock())
 
     def test_invalid_profile(self):
         with self.assertRaises(ValueError):