Calculating travel statistics based on CSV data I took a code test for acme transport company and was rejected. That's ok. I'm still learning how to code. I didn't get much feedback so that's a bit frustrating. Can anyone here provide tips on how to optimize this python code? I thought that I had done a pretty good job with it, had the correct answers, etc. But apparently there are some fatal flaws, at least according to acme transport co!

Here's the challenge:

Parse a text file so that we know the following:


how many cars were dropped in total?
which car had the longest single trip? How long was the trip?
what car had the longest cumulative distance? How long was that.


The text file is formatted like this:


timestamp, Integer, The time in seconds since the start of the simulation
id, String, The id of the associated vehicle, e.g. JK5T
event_type, String The type of the event is one of START_RIDE, END_RIDE, DROP
x, Double, The x coordinate of the location of where the event happened in the simulation
y, Double, The y coordinate of the location of where the event happened in the simulation
user_id, Integer, The id of the associated user or NULL if the event does not have an associated user


The first few lines of the file look like this:

302,OPN1,DROP,33.28280226855614,-87.88162047789511,NULL
305,OPN1,START_RIDE,33.28280226855614,-87.88162047789511,4204
419,G2I0,DROP,-28.367411419413685,23.567421582328606,NULL
426,OPN1,END_RIDE,7.563016232237587,91.97004351346015,4204
428,LQBF,DROP,-50.50579634246066,-54.53980771216895,NULL
441,5HQH,DROP,-70.05156581770649,36.8358644984674,NULL


For my code I import a library called shapely that helps determine distances:

import csv
from shapely.geometry import Point
from collections import defaultdict



def main():
        myDict = defaultdict(dict)
        aDict = {}
        totalDrops = 0
        maxCar = None
        with open('events.txt') as f:
            reader = csv.reader(f)
            for row in reader:
                if row[2] == 'DROP':
                    totalDrops += 1 
                    myDict[row[1]]['drop'] = Point(float(row[3]), float(row[4]))
                if row[2] == 'START_RIDE':
                    myDict[row[1]]['start_ride'] = Point(float(row[3]), float(row[4]))
                if row[2] == 'END_RIDE':
                    singleTripDistance = myDict[row[1]]['drop'].distance(Point(float(row[3]), float(row[4])))
                    distanceTraveled = myDict[row[1]]['start_ride'].distance(Point(float(row[3]), float(row[4])))
                    aDict[row[1]] = singleTripDistance
                    if not maxCar:
                        maxCar = row[1]
                    if 'allDistances' not in myDict[row[1]]:
                        myDict[row[1]]['allDistances'] = distanceTraveled
                    else:
                        myDict[row[1]]['allDistances'] += distanceTraveled
                    if  myDict[row[1]]['allDistances'] &gt; myDict[maxCar]['allDistances']:
                        maxCar = row[1]
            maxSingleTrip = max(zip(aDict.values(), aDict.keys()))
            print('There are ' + '' + str(totalDrops) + ' ' + 'total Drops.' + '\n' + 'The Car that ends up furthest away from its drop is:' + ' ' + str(maxSingleTrip[1]) + ' ' + 'and the distance is:' + ' ' + str(maxSingleTrip[0]) + '\n' + 'The Car with the highest total distance traveled is:' + ' ' + maxCar + ', ' + 'and the total distance is:' + ' ' +  str(myDict[maxCar]['allDistances']))
main()
What you did well

You used the csv module for parsing the CSV file, and you processed it in one pass.  Your code is therefore reasonably efficient, and I wouldn't worry much about its performance.

Bugs

Your code reports "The Car that ends up furthest away from its drop is: …", but that is, in my interpretation, not the same as the longest single trip, which is what you were asked to calculate.

Concerns

Your data analysis code is mingled with the CSV parsing.  That makes the code hard to read and hard to maintain.

The deeply nested dictionaries are hard to understand.  What does myDict store?  Its purpose is not apparent from its name.  It's a two-level dictionary, where the first-level key is a vehicle_id, the second-level key is some attribute of that vehicle (the 'drop' coordinate, the 'start_ride' coordinate, or the 'allDistances' running total).  You would be better off with three independent dictionaries.  Better yet, as I've done below, define an Itinerary class instead.

What's aDict?  It keeps track of the distance from each car's drop point to the endpoint of its last trip.  Again, the variable name does not help.

The print() call is hard to read, in an excessively long line of code, with lots of str() casts and unnecessary string concatenations.  In this situation, I would recommend f-strings (introduced in Python 3.6), or str.format().

Suggested solution

This solution reports the car that ends up the furthest from its drop point, as you did, rather than the longest single trip as requested.

The Shapely library is overkill; subclassing namedtuple with a .distance() method is all I need for a handy Point class.  Euclidean distances can be calculated using math.hypot.

If you look at the data, you'll see that each car acts as a state machine.  It is initialized with a DROP event.  Thereafter, it alternates between START_RIDE and END_RIDE events.  No traveling is allowed when a START_RIDE occurs; the car is always the same place as its preceding DROP or END_RIDE coordinates.  In my Itinerary class, I've kept track of the .state of the car so that I can perform certain assertions, but it's only essential to keep track of the coordinates at each DROP and END_RIDE event.

The itineraries() function parses the CSV into one simple data structure: a dictionary mapping vehicle IDs to their itineraries.  I've chosen to use fileinput.input() to accept the file on the command line or through sys.stdin without hard-coding the filename.

The data analysis is accomplished using built-in functions len() and max() with generator expressions.  That is more expressive and less cumbersome than maintaining several intermediate results each time an END_RIDE event occurs.

from collections import namedtuple
import csv
import fileinput
from math import hypot
from operator import attrgetter

class Point(namedtuple('Point', 'x y')):
    def distance(self, other):
        """Euclidean distance between two points."""
        return hypot(self.x - other.x, self.y - other.y)

class Itinerary:
    def __init__(self, car, x, y):
        self.car = car
        self.coords = [Point(x, y)]
        self.state = 'DROP'
        self.cumul_dist = 0

    def start_ride(self, x, y):
        assert self.state in ('DROP', 'END_RIDE'), "Restarting unfinished ride"
        assert Point(x, y) == self.coords[-1], "Teleportation occurred"
        self.state = 'START_RIDE'

    def end_ride(self, x, y):
        assert self.state == 'START_RIDE', "Ending a ride that hasn't started"
        self.coords.append(Point(x, y))
        self.state = 'END_RIDE'
        self.cumul_dist += self.coords[-1].distance(self.coords[-2])

    @property
    def final_dist(self):
        return self.coords[-1].distance(self.coords[0])

def itineraries(f):
    cars = {}
    for timestamp, car, event_id, x, y, user_id in csv.reader(f):
        if event_id == 'DROP':
            assert car not in cars, "Dropping the same car twice"
            cars[car] = Itinerary(car, float(x), float(y))
        elif event_id == 'START_RIDE':
            cars[car].start_ride(float(x), float(y))
        elif event_id == 'END_RIDE':
            cars[car].end_ride(float(x), float(y))
    return cars

def main():
    cars = itineraries(fileinput.input()).values()
    drops = len(cars)
    furthest = max(cars, key=attrgetter('final_dist'))
    most_traveled = max(cars, key=attrgetter('cumul_dist'))
    print(f"""There are {drops} total drops.
The car that ends up furthest from its drop is {furthest.car}, at distance {furthest.final_dist}.
Car {most_traveled.car} travels the longest cumulative distance: {most_traveled.cumul_dist}.""")

if __name__ == '__main__':
    main()