# Get nearest major city of an image (using EXIF GPS data)

I am writing a program that will take images in a folder, get the GPS Coordinates (from the EXIF data), then return the closest "major city".

(I define a major city as one with an airport, so technically I'm taking the GPS coords of an image, and finding the closest airport and returning that city)

I've looked at this a lot and think I have it pretty efficient. However, aside from any other general or specific comments, and curious on the following points:

1. Naming could be better on the functions
2. Are the doc strings good, or are they too broad/obvious?
3. For the constants I declare up top, should I use enum?
4. Am I logging correctly?
5. Are my try/except options in good order? I feel that the setting of None/False is a little repetitive, and not too DRY - can that be improved?

5b) How should I handle exceptions I don't know about, or haven't already handled?

6. Specifically, is airports_by_country() a good/necessary function or should I just directly filter the DF in the original funtion that calls it?

7. Does the "order" of the functions matter?

find_city.py:

import os
import logging
import _pickle as cPickle

from GPSPhoto import gpsphoto
import pandas as pd
from geopy.geocoders import Nominatim
from geopy import distance

THIS_PY_LOC = os.path.dirname(os.path.abspath(__file__))
IMAGE_FOLDER = "Images/"
IMAGE_INFO_DICT = "image_data_dict.txt"

# Airport info from https://openflights.org/data.html
AIRPORT_INFO = os.path.join(THIS_PY_LOC, "airports.csv")

LOG_FILENAME = str(os.path.basename(__file__)) + '.log'
logging.basicConfig(filename=LOG_FILENAME, level=logging.DEBUG)

def image_paths(top_folder):
"""
With the top_folder, return all files in that folder with path.
This will NOT look in subdirectories.
https://docs.python.org/3/library/os.path.html#os.path.isfile
"""
paths = []
for filename in os.listdir(IMAGE_FOLDER):
_file = os.path.join(IMAGE_FOLDER, filename)
if os.path.isfile(_file):
paths.append(_file)
return paths

def return_image_data_from_list(image_list):
"""
Takes a list of Image Paths, and will
return Lat/Long data
"""
info = {}
geolocator = Nominatim(user_agent="my-application")
for img in image_list:
data = gpsphoto.getGPSData(img)
try:
_coords = str(data['Latitude']) + "," + str(data['Longitude'])
locale = geolocator.reverse(_coords, timeout=20)
if full_info is None:
full_info = locale
"gps": {"latitude": data['Latitude'],
"longitude": data['Longitude']},
"location unknown": False}

except KeyError as err:
logging.info(img + " had error: " + str(err))
"country": None,
"gps": {"latitude": None,
"longitude": None},
"location unknown": True}
return info

def airports_by_country(airport_df, country):
"""
Using a dataframe, extract only the rows where the
airport is in a particular country and return just that
information as a DF
"""
airports = airport_df[airport_df['Country'] == country]
return airports

"""
Using Pickle, this will check to see if the image dictionary
which has the address/GPS data exists. If so, use that as the
dictionary.  Otherwise, create it.  This will help cut down
on the requests, as if the dict with the info exists, we will
just use that.
"""
try:
with open(IMAGE_INFO_DICT, 'rb') as dict_items_open:
logging.info("Pickle dictionary found, using that.")
except (EOFError, FileNotFoundError) as err:
logging.info("Creating Pickle dictionary.")
images = image_paths(IMAGE_FOLDER)
image_gps_info = return_image_data_from_list(images)
with open(IMAGE_INFO_DICT, 'wb') as dict_items_save:
cPickle.dump(image_gps_info, dict_items_save)
return image_gps_info

def get_distance_to_airport(org_lat, org_lon, country_df):
"""
Takes the origin latitutde and longitude
and searches through a Pandas DATAFRAME that has
columns "Latitude" and "Longitude" and finds the closest
coordinate pair and returns that pair.

Uses Vincenty distance, per
https://stackoverflow.com/a/43211266
"""
closest_airport = {'distance': 1000000000000000000}
for row in country_df.itertuples():
row_lat = row.Latitude
row_lon = row.Longitude
# dist = gpxpy.geo.haversine_distance(float(org_lat), float(org_lon),
#                                    float(row_lat), float(row_lon))
vin_dist = distance.vincenty(
(float(org_lat), float(org_lon)),
(float(row_lat), float(row_lon))).miles
if vin_dist < closest_airport['distance']:
closest_airport['distance'] = vin_dist
closest_airport['airport'] = row.Name
closest_airport['city'] = row.City
return closest_airport

def update_dict_with_airport(primary_dict, airport_df):
"""
This takes the original dictionary and adds in the
closest airport information
"""
for key, val in primary_dict.items():
try:
_country = val["country"]
img_gps = val['gps']
country_df = airports_by_country(airport_df, _country)
closest_airport = get_distance_to_airport(img_gps['latitude'],
img_gps['longitude'],
country_df)
primary_dict[key]['closest city'] = closest_airport['city']
primary_dict[key]['airport'] = closest_airport['airport']
primary_dict[key]['miles_to_airport'] = closest_airport['distance']
except KeyError:
primary_dict[key]['closest city'] = None
primary_dict[key]['airport'] = None
primary_dict[key]['distance_to_airport'] = None
return primary_dict

def image_data_from_folder(fldr):
"""
This is for running externally.
Pass in a folder location, and this will
check all the images in that folder for the information
and return that in a dictionary.
"""
full_image_dict = update_dict_with_airport(image_gps_info, airport_df)
unknown_locs = unknown_coords(full_image_dict)
logging.info("UNKNOWN IMAGE COORDS: " + str(unknown_locs))
logging.info("-----------------------------------------------------------")
return full_image_dict

def unknown_coords(image_dict):
"""
Takes a dictionary and returns the key name for
files where 'unknown' key is False.
"""
_files = []
for image, vals in image_dict.items():
# print(image, "\n", vals)
if not image_dict[image]["location unknown"]:
_files.append(image)
return _files

def main():
full_image_dict = image_data_from_folder(IMAGE_FOLDER)
for key, val in full_image_dict.items():
print(key, "\n", val)

if __name__ == '__main__':
main()


A final comment is that I'd like this to work well as an import to another file. So later on, if I have a folder of images, I could import find_city as fc then do image_city_info = fc.image_data_from_folder("/myFolder")

## General

Going through your list of concerns:

1. the function names are not crazy, though you could definitely benefit from implementing type hints;
2. the doc strings should specifically describe all parameters and return values;
3. no, I don't see enum being useful here;
4. logging seems fine to me;
5. Your exceptions are fine. The DRY concern is... not really that big of a deal. You could go out of your way to define a tuple of default keys needed for a dictionary, but it's more hassle than it's worth. About unhandled exceptions: let them stay exceptional. At the most, you may want a top-level except Exception in your main that logs exceptions that fall through.
6. Seems fine, though I don't understand your data well enough to speak authoritatively
7. Yes. Generally, they should be listed in order of dependency (callee first, caller later). This is already what you have.

at the top, probably #!/usr/bin/env python3 .

## Use pathlib instead of path.join

Something like AIRPORT_INFO = Path(THIS_PY_LOC) / 'airports.csv'

## Use f-strings instead of concatenates

Something like LOG_FILENAME = f'{os.path.basename(__file__)}.log'

Also seen: str(data['Latitude']) + "," + str(data['Longitude']) becomes

f'{data["Latitude"]},{data["Longitude"]}'

## Use generators

def image_paths(top_folder):
for filename in os.listdir(IMAGE_FOLDER):
_file = Path(IMAGE_FOLDER) / filename
if _file.exists():
yield _file


This also applies to your unknown_coords.

## Use update on dicts

i.e.

        primary_dict[key]['closest city'] = None
primary_dict[key]['airport'] = None
primary_dict[key]['distance_to_airport'] = None


becomes

primary_dict[key].update({
'closest city': None,
'airport': None,
'distance_to_airport': None
})


## Use scandir instead of listdir

According to the docs:

The scandir() function returns directory entries along with file attribute information, giving better performance for many common use cases.

So use that in your image_paths function. In fact... reading that function again, why are you looping at all? This can be a one-liner - just call scandir(). I'm not sure why you are checking for existence of the files - do you not trust the values returned by listdir?

• Thanks very much, I appreciate all the comments! While I'm also googling around for more details, can you add a little more info on why to use pathlib instead of path.join and the generators? I use listdir because later on, I plan on checking for the filetype of each file and see if it's an image media type. (In the event I scan a folder of images, video, xlsx, etc. so I don't include stuff that couldn't have GPS. – BruceWayne Jun 10 '19 at 16:33
• Pathlib because it more or less looks nicer during path construction, plus it returns a more useful object instead of just a string. Generators because they take up less memory, and reduce complexity of your application. – Reinderien Jun 10 '19 at 20:45
• For the generator, to make sure I understand, I'll then have to do for img in list(image_list):, correct? Since image_list would be the returned generator? – BruceWayne Jun 11 '19 at 16:32
• It's even easier - you don't need to convert it into a list unless (a) you need to iterate twice, or (b) you need it all in memory for random access or something else. In your case, for one for loop, just write for img in image_list. – Reinderien Jun 11 '19 at 17:16