Python methods from csv

Question

I am working on an assignment where I create "instances" of cities using rows in a .csv, then use these instances in methods to calculate distance and population change. Creating the instances works fine (using steps 1-4 below), until I try to call printDistance:

##Step 1. Open and read CityPop.csv
with open('CityPop.csv', 'r', newline='') as f:
try:
    reader = csv.DictReader(f)
    ##Step 2. Create "City" class
    class City:
        ##Step 3. Use _init method to assign attribute values
        def __init__(self, row, header):
            self.__dict__ = dict(zip(header, row))

            ##Step 4. Create "Cities" list
            data = list(csv.reader(open('CityPop.csv')))
            instances = [City(i, data[0]) for i in data[1:]]

            ##Step 5. Create printDistance method within "Cities" class  
            def printDistance(self, othercity, instances):
                dist=math.acos((math.sin(math.radians(self.lat)))*(math.sin(math.radians(othercity.lat)))+(math.cos(math.radians(self.lat)))*(math.cos(math.radians(othercity.lat)))*(math.cos(math.radians(self.lon-othercity.lon)))) * 6300 (self.lat, self.lon, othercity.lat, othercity.lon)

When I enter instances[0].printDistance(instances1) in the shell, I get the error:

 `NameError: name 'instances' is not defined`

Is this an indentation problem? Should I be calling the function from within the code, not the shell?

Answer 1

This is not so much an indentation problem, but more of a general code structure problem. You're nesting a lot:

• All the actual work on an incredibly long line (with errors)
• Inside of function (correctly) printDistance
• Inside of a constructor __init__
• Inside of a class definition (correctly) City
• Inside of a try block
• Inside of a with block

I think this is what you are trying to do:

• create a class City, which can print the distance of itself to other cities
• generate a list of these City objects from a .csv that somehow has both distances and population (you should probably provide an example of data)
• do so in a fault-tolerant and clean way (hence the try and the with)

The reason your instances isn't working is because, unlike you think, it's probably not being created correctly, or at least not in the correct context. And it certainly won't be available to you on the CLI due to all of the nesting.

There's a number of blatant bugs in your code:

• What's the (self.lat, self.lon, othercity.lat, othercity.lon) at the end of the last line?
• Why are you opening the file for reading twice? You're not even using the first reader
• You are bluntly assigning column headers from a .csv as object attributes, but are misspelling their use (lat instead of latitude and lon instead of longitude)

It looks a bit like a lot of code found in various places got pasted together into one clump - this is what it looks like when cleaned up:

import csv
import math


class City:
    def print_distance(self, other_city):
        print(f'{self.city} to {other_city.city}')
        # what a mess...
        print(math.acos(
            (math.sin(math.radians(float(self.latitude)))) * (math.sin(math.radians(float(other_city.latitude)))) + (
                math.cos(math.radians(float(self.latitude)))) * (math.cos(math.radians(float(other_city.latitude)))) * (
                math.cos(math.radians(float(self.longitude) - float(other_city.longitude))))) * 6300)

    def __init__(self, values, attribute_names):
        # this is *nasty* - much better to add the attributes explicitly, but left as original
        # also, note that you're reading strings and floats here, but they are all stored as str
        self.__dict__ = dict(zip(attribute_names, values))


with open('CityPop.csv', 'r', newline='') as f:
    try:
        reader = csv.reader(f)
        header = next(reader)
        cities = [City(row, header) for row in reader]

        for city_1 in cities:
            for city_2 in cities:
                city_1.print_distance(city_2)
    except Exception as e:
        print(f'Apparently were doing something with this error: {e}')

Note how print_distance is now a method of City, which is called on each instance of City in cities (which is what I renamed instances to).

Now, if you are really trying, this makes more sense:

import csv
import math


class City:
    def print_distance(self, other_city):
        print(f'{self.name} to {other_city.name}')
        # not a lot better, but some at least
        print(
            math.acos(
                math.sin(math.radians(self.lat)) *
                math.sin(math.radians(other_city.lat))
                +
                math.cos(math.radians(self.lat)) *
                math.cos(math.radians(other_city.lat)) *
                math.cos(math.radians(self.lon - other_city.lon))
            ) * 6300
        )

    def __init__(self, lat, lon, name):
        self.lat = float(lat)
        self.lon = float(lon)
        self.name = str(name)


try:
    with open('CityPop.csv', 'r', newline='') as f:
        reader = csv.reader(f)
        header = next(reader)
        cities = [City(lat=row[1], lon=row[2], name=row[4]) for row in reader]

        for city_1 in cities:
            for city_2 in cities:
                city_1.print_distance(city_2)
except FileNotFoundError:
    print(f'Could not find the input file.')

Note the cleaned up computation, the catching of an error that could be expected to occur (with the with insides the try block) and a proper constructor that assigns what it needs with the correct type, while the reader decides which fields go where.

Finally, as a bonus: nobody should be writing distance calculations like this. Plenty libraries exist that do a much better job of this, like GeoPy. All you need to do is pip install geopy to get it and then you can use this:

import csv
import geopy.distance


class City:
    def calc_distance(self, other_city):
        return geopy.distance.geodesic(
            (self.lat, self.lon), 
            (other_city.lat, other_city.lon)
        ).km

    def __init__(self, lat, lon, name):
        self.lat = float(lat)
        self.lon = float(lon)
        self.name = str(name)


try:
    with open('CityPop.csv', 'r', newline='') as f:
        reader = csv.reader(f)
        header = next(reader)
        cities = [City(lat=row[1], lon=row[2], name=row[4]) for row in reader]

        for city_1 in cities:
            for city_2 in cities:
                print(city_1.calc_distance(city_2))
except FileNotFoundError:
    print(f'Could not find the input file.')

Note that I moved the print out of the method as well, since it makes more sense to calculate in the object and print outside it. The nice thing about all this is that the calculation now uses a proper geodesic (WGS-84) to do the calculation and the odds of math errors are drastically reduced. If you must use a simple sphere, the library has functions for that as well.

Answer 2

Nested functions must not contain self as parameter because they are not member functions. Class cannot pass instance variables to them. You are infact passing the same self from parent to child function.

Also you must not nest constructor, this is only for initiation purpose. Create a separate method indeed.

And try creating instance variable inside the constructor, and that is what init for !

self.instances = [self.getInstance(i, data[0]) for i in data[1:]]

Also create seperate function for instantiation

@classmethod
def getInstance(cls,d1,d2):
    return cls(d1,d2)