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

Question

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)
            full_info = geolocator.geocode(locale, addressdetails=True)
            # address = full_info.raw["address"]
            if full_info is None:
                full_info = locale
            info[img] = {"address": locale,
                         "country": full_info.raw['address']['country'],
                         "gps": {"latitude": data['Latitude'],
                                 "longitude": data['Longitude']},
                         "location unknown": False}

        except KeyError as err:
            logging.info(img + " had error: " + str(err))
            info[img] = {"address": None,
                         "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


def load_image_dict(image_folder=IMAGE_FOLDER):
    """
    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.")
            image_gps_info = cPickle.load(dict_items_open)
    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.
    """
    image_gps_info = load_image_dict(fldr)
    airport_df = pd.read_csv(AIRPORT_INFO)
    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")

Answer 1

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.

Add a shebang

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?