From 09e240fc61cd5b71fce4e3d2f7123fa8bdd937ee Mon Sep 17 00:00:00 2001 From: Natalie Schultz <90212258+nataliejschultz@users.noreply.github.com> Date: Mon, 9 Oct 2023 16:03:50 -0600 Subject: [PATCH] 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 | 95 ++++++++++++------- emission/integrationTests/docker-compose.yml | 5 +- .../net/ext_service/geocoder/nominatim.py | 15 +-- 3 files changed, 73 insertions(+), 42 deletions(-) diff --git a/emission/individual_tests/TestNominatim.py b/emission/individual_tests/TestNominatim.py index ced297a18..b9b19c77f 100644 --- a/emission/individual_tests/TestNominatim.py +++ b/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,6 +101,7 @@ 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") @@ -72,32 +109,20 @@ def test_geocode(self): 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() \ No newline at end of file diff --git a/emission/integrationTests/docker-compose.yml b/emission/integrationTests/docker-compose.yml index 5e37ac369..00db0ad6c 100644 --- a/emission/integrationTests/docker-compose.yml +++ b/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 diff --git a/emission/net/ext_service/geocoder/nominatim.py b/emission/net/ext_service/geocoder/nominatim.py index 3898b9543..e20e093a3 100644 --- a/emission/net/ext_service/geocoder/nominatim.py +++ b/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)