The big one!

I am almost positive I have the URL formatting how it is supposed to be. All of the tests are passing locally. Here's what I've done:

TestNominatim.py:
 - Created a new module that sets the query URL to be used in nominatim.py and reloads nominatim.py. This allows the user to call the various nominatim services anywehre in the code, without modifying nominatim.py.

- added setup function for consistency with other tests

docker-compose: added query URLs as environment variables for the web-server service.

nominatim.py: set the nominatim_query_url as an environment variable in a more simple way.

emission/individual_tests/TestNominatim.py

@@ -6,38 +6,64 @@
 standard_library.install_aliases()
 from builtins import *
 import unittest
+import importlib
 import os
 from emission.core.wrapper.trip_old import Coordinate
 import requests
 import bin.createfakeplace as bc 
-import emission.core.wrapper.entry as ecwe
-import emission.net.ext_service.geocoder.nominatim as eco
 import emission.analysis.intake.cleaning.clean_and_resample as clean
+import emission.net.ext_service.geocoder.nominatim as eco
 
-#Sets OPENSTREETMAP_QUERY_URL to the environment variable.
-OPENSTREETMAP_QUERY_URL_env = os.environ.get("OPENSTREETMAP_QUERY_URL")
-
-OPENSTREETMAP_QUERY_URL = (
-    OPENSTREETMAP_QUERY_URL_env if OPENSTREETMAP_QUERY_URL_env is not None 
-    else eco.OPENSTREETMAP_QUERY_URL
-)
+#Setting query URLs
+OPENSTREETMAP_QUERY_URL = os.environ.get("OPENSTREETMAP_QUERY_URL")
+GEOFABRIK_QUERY_URL = os.environ.get("GEOFABRIK_QUERY_URL")
+NOMINATIM_CONTAINER_URL = os.environ.get("NOMINATIM_CONTAINER_URL")
 
-GFBK = os.environ.get("GFBK_KEY")
+# GFBK = os.environ.get("GFBK_KEY")
 
-GEOFABRIK_QUERY_URL = (
-    "https://geocoding.geofabrik.de/{}".format(GFBK) if GFBK is not None
-    else print("No key available")
-)
+# GEOFABRIK_QUERY_URL = (
+#     "https://geocoding.geofabrik.de/{}".format(GFBK) if GFBK is not None
+#     else print("No key available")
+# )
 class NominatimTest(unittest.TestCase):
     maxDiff = None
 
-    #Basic query to check that nominatim and geofabrik are returning the same data.
+    def setUp(self) -> None:
+        return super().setUp()
+
+    #When a nominatim service is called, we set the value of the NOMINATIM_QUERY_URL environment variable in nominatim.py and re-load the module.
+    def nominatim(service):
+        if service == "container":
+            os.environ["NOMINATIM_QUERY_URL"] = NOMINATIM_CONTAINER_URL
+            importlib.reload(eco)
+        elif service == "geofabrik":
+            os.environ["NOMINATIM_QUERY_URL"] = GEOFABRIK_QUERY_URL
+            importlib.reload(eco)
+        elif service == "OSM":
+            os.environ["NOMINATIM_QUERY_URL"] = OPENSTREETMAP_QUERY_URL
+            importlib.reload(eco)
+
+    #Basic query to check that OSM, the Rhode Island Container, and geofabrik are returning the same data.
     def test_geofabrik_and_nominatim(self):
-        OPENSTREETMAP_result = requests.get(OPENSTREETMAP_QUERY_URL + "/reverse?lat=41.8239891&lon=-71.4128343&format=json").json()
-        geofabrik_result = requests.get(GEOFABRIK_QUERY_URL + "/reverse?lat=41.8239891&lon=-71.4128343&format=json").json()
+        lat, lon = 41.8239891, -71.4128343
+        NominatimTest.nominatim("container")
+        container_result = eco.Geocoder.get_json_reverse(lat,lon)
+        NominatimTest.nominatim("OSM")
+        osm_result = eco.Geocoder.get_json_reverse(lat,lon)
+        NominatimTest.nominatim("geofabrik")
+        geofabrik_result = eco.Geocoder.get_json_reverse(lat,lon)
         key_list = ['osm_id', 'boundingbox']
         for k in key_list:
-            self.assertEqual(OPENSTREETMAP_result[k], geofabrik_result[k])
+            self.assertEqual(osm_result[k], geofabrik_result[k])
+            self.assertEqual(container_result[k], geofabrik_result[k])
+
+    # #Compares the result of a call to OSM with a call to the container. 
+    # def test_nominatim_api(self):
+    #     nominatim_url = "http://nominatim.openstreetmap.org/reverse?lat=41.832942092439694&lon=-71.41558148857203&format=json"
+    #     nominatim_result_raw = requests.get(nominatim_url)
+    #     nominatim_result = nominatim_result_raw.json()['display_name'][0:70]
+    #     container_result = eco.Geocoder.reverse_geocode(41.832942092439694, -71.41558148857203)[0:70]
+    #     self.assertEqual(nominatim_result, container_result)
 
     #Checks the display name generated by get_filtered_place in clean_and_resample.py, which creates a cleaned place from the fake place
     # and reverse geocodes with the coordinates.
@@ -50,13 +76,23 @@ def test_get_filtered_place(self):
 
     #Testing make_url_geo, which creates a query URL from the input string. 
     def test_make_url_geo(self):
-        expected_result = OPENSTREETMAP_QUERY_URL + "/search?q=Providence%2C+Rhode+Island&format=json"
+        expected_result = GEOFABRIK_QUERY_URL + "/search?q=Providence%2C+Rhode+Island&format=json"
+        NominatimTest.nominatim("geofabrik")
         actual_result = eco.Geocoder.make_url_geo("Providence, Rhode Island")
         self.assertEqual(expected_result, actual_result)
 
+        #Testing make_url_reverse, which creates a query url from a lat and lon.
+    def test_make_url_reverse(self):
+        NominatimTest.nominatim("geofabrik")
+        lat, lon = 41.8239891, -71.4128343
+        expected_result = GEOFABRIK_QUERY_URL + (f"/reverse?lat={lat}&lon={lon}&format=json")
+        actual_result = (eco.Geocoder.make_url_reverse(41.8239891, -71.4128343))
+        self.assertEqual(expected_result, actual_result)
+
     #Testing get_json_geo, which passes in an address as a query. Compares three select k,v pairs in the results. 
     def test_get_json_geo(self):
-        expected_result = {'place_id': 132490, 'licence': 'Data © OpenStreetMap contributors, ODbL 1.0. https://osm.org/copyright', 'osm_type': 'way', 'osm_id': 141567710, 'boundingbox': ['41.8325787', '41.8332278', '-71.4161848', '-71.4152064'], 'lat': '41.8330097', 'lon': '-71.41568124868104', 'display_name': 'State of Rhode Island Department of Administration, 1, Park Street, Downtown, Providence, Providence County, 02908, United States', 'class': 'building', 'type': 'civic', 'importance': 1.75001}
+        NominatimTest.nominatim("geofabrik")
+        expected_result = {'place_id': 132490, 'licence': 'Data © OpenStreetMap contributors, ODbL 1.0. https://osm.org/copyright', 'osm_type': 'way', 'osm_id': 141567710, 'boundingbox': ['41.8325787', '41.8332278', '-71.4161848', '-71.4152064'], 'lat': '41.8330097', 'lon': '-71.41568124868104', 'display_name': 'State of Rhode Island Department of Administration, 1, Park Street, Downtown, Providence, Providence County, Rhode Island, 02908, United States', 'class': 'building', 'type': 'civic', 'importance': 1.75001}
         actual_result = eco.Geocoder.get_json_geo("State of Rhode Island Department of Administration, 1, Park Street, Downtown, Providence, Providence County, 02908, United States")[0]
         key_list = ['osm_id', 'boundingbox', 'display_name']
         for k in key_list:
@@ -65,39 +101,28 @@ def test_get_json_geo(self):
     #Testing the geocode function, which passes in an address and gets latitude and longitude.
     # Test creates instance of coordinates using coordinate class. Getting lat and lon of the coordinate using get_lat and get_lon methods from the class.
     def test_geocode(self):
+        NominatimTest.nominatim("geofabrik")
         expected_result_lon = Coordinate(41.8239891, -71.4128343).get_lon()
         expected_result_lat = Coordinate(41.8239891, -71.4128343).get_lat()
         actual_result = eco.Geocoder.geocode("Providence, Rhode Island")
         actual_result_lon = actual_result.get_lon()
         actual_result_lat = actual_result.get_lat()
         self.assertEqual(expected_result_lon, actual_result_lon)
         self.assertEqual(expected_result_lat, actual_result_lat)
-
-    #Testing make_url_reverse, which creates a query url from a lat and lon
-    def test_make_url_reverse(self):
-        expected_result = OPENSTREETMAP_QUERY_URL + "/reverse?lat=41.8239891&lon=-71.4128343&format=json"
-        actual_result = (eco.Geocoder.make_url_reverse(41.8239891, -71.4128343))
-        self.assertEqual(expected_result, actual_result)
 
     #Testing get_json_reverse, which reverse geocodes from a lat and lon. Tested result was modified to only look at the name returned with the coordinates, rather than the entire dictionary.
     def test_get_json_reverse(self):
+        NominatimTest.nominatim("geofabrik")
         expected_result = "Providence City Hall"
         actual_result = eco.Geocoder.get_json_reverse(41.8239891, -71.4128343)["display_name"].split(",")[0]
         self.assertEqual(expected_result, actual_result)
 
     #Testing reverse_geocode, which reverse geocodes from a lat and lon and returns only the display name. 
     def test_reverse_geocode(self):
-        expected_result = "Portugal Parkway, Fox Point, Providence, Providence County, 02906, United States"
+        NominatimTest.nominatim("geofabrik")
+        expected_result = "Portugal Parkway, Fox Point, Providence, Providence County, Rhode Island, 02906, United States"
         actual_result = eco.Geocoder.reverse_geocode(41.8174476, -71.3903767)
         self.assertEqual(expected_result, actual_result)
 
-    #Compares the result of a hard-coded nominatim call with our container. 
-    def test_nominatim_api(self):
-        nominatim_url = "http://nominatim.openstreetmap.org/reverse?lat=41.832942092439694&lon=-71.41558148857203&format=json"
-        nominatim_result_raw = requests.get(nominatim_url)
-        nominatim_result = nominatim_result_raw.json()['display_name'][0:70]
-        docker_result = eco.Geocoder.reverse_geocode(41.832942092439694, -71.41558148857203)[0:70]
-        self.assertEqual(nominatim_result, docker_result)
-
 if __name__ == '__main__':
     unittest.main()

emission/integrationTests/docker-compose.yml

@@ -8,8 +8,11 @@ services:
       - nominatim
     environment:
       - DB_HOST=db
-      - OPENSTREETMAP_QUERY_URL=http://rhodeisland-nominatim:8080
       - GFBK_KEY=$GFBK_KEY
+      - GEOFABRIK_QUERY_URL=https://geocoding.geofabrik.de/$GFBK_KEY
+      - OPENSTREETMAP_QUERY_URL=https://nominatim.openstreetmap.org
+      - NOMINATIM_CONTAINER_URL=http://rhodeisland-nominatim:8080
+
     volumes:
       # specify the host directory where the source code should live
       # If this is ~/e-mission-server-docker, then you can edit the files at

emission/net/ext_service/geocoder/nominatim.py

@@ -13,15 +13,18 @@
 
 from emission.core.wrapper.trip_old import Coordinate
 try:
-    OPENSTREETMAP_QUERY_URL_env = os.environ.get("OPENSTREETMAP_QUERY_URL", "")
-    logging.info(f"OPENSTREETMAP_QUERY_URL_env: {OPENSTREETMAP_QUERY_URL_env}")
-    OPENSTREETMAP_QUERY_URL = OPENSTREETMAP_QUERY_URL_env if OPENSTREETMAP_QUERY_URL_env != "" else "http://nominatim.openstreetmap.org"
-    print("Open Street Map URL not configured, defaulting to nominatim:") if OPENSTREETMAP_QUERY_URL == "http://nominatim.openstreetmap.org" else print("Open Street Map URL configured!") 
+    NOMINATIM_QUERY_URL = os.environ.get("NOMINATIM_QUERY_URL")
+    # logging.info(f"NOMINATIM_QUERY_URL: {NOMINATIM_QUERY_URL}")
+    print("URL Configured:", NOMINATIM_QUERY_URL)
+
+    if NOMINATIM_QUERY_URL is None:
+        raise Exception("Nominatim query url not configured")
 except:
     print("URL not configured, place decoding must happen on the client")
 
 class Geocoder(object):
 
+
     def __init__(self):
         pass
 
@@ -31,7 +34,7 @@ def make_url_geo(cls, address):
             "q" : address,
             "format" : "json"
         }
-        query_url = OPENSTREETMAP_QUERY_URL + "/search?"
+        query_url = NOMINATIM_QUERY_URL + "/search?"
         encoded_params = urllib.parse.urlencode(params)
         url = query_url + encoded_params
         logging.debug("For geocoding, using URL %s" % url)
@@ -59,7 +62,7 @@ def make_url_reverse(cls, lat, lon):
             "format" : "json"
         }
 
-        query_url = OPENSTREETMAP_QUERY_URL + "/reverse?"
+        query_url = NOMINATIM_QUERY_URL + "/reverse?"
         encoded_params = urllib.parse.urlencode(params)
         url = query_url + encoded_params
         logging.debug("For reverse geocoding, using URL %s" % url)