2
\$\begingroup\$

This project was completed as part of an interview challenge.

The feedback I received was:

  1. The code is neither optimized nor does it follow generally accepted paradigms, naming conventions or best practices

  2. There are a lot of nested loops and if/else clauses that could be simplified, he has redundant import statements in the middle of function declarations, he uses JavaScript style string interpolation.

Could anyone give me some examples of what specifically I could change to implement these suggestions?

For task description see the doc-string under the cleaner method.

"""

To run any of the methods below just uncomment the method call at the very bottom of 
the script
"""


likes = open('filepath' + 'likes.csv')
fileWritePath = 'filepath' + 'dct'
likeDir =  'filepath' + 'scrubLikes' 
likesToAddCSV = 'filepath' + 'newLikes.csv'
newLikeDir = 'filepath' + 'newLikes' 
rawClusters = 'filepath' + 'rawClusters.csv'
rawVectors = 'filepath' + 'rawVectors.csv'  



def cleaner(likes):
    import copy
    '''
    estimated run time for 170k users: 3min
    this method takes a given csv format datasheet of noisy facebook likes.
    data is scrubbed row by row removing 'likes' that are not useful
    data is parsed into manageable size specified files.
    if more data is continuously added method will just keep adding new files
    if more data is added at a later time chosing a new folder to put it in would
    work best so that the update method can add it to existing counts instead of 
    starting over
    '''

    dct = [0]
    fileNum = 0
    #initializes naming scheme for self-numbering files
    fileSize = 30000
    #sets file size to 30000 userId's
    alphanum = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 $%@-'
    userCount = 0

    for rows in likes: 
        repeatCheck = []        
        userCount += 1
        userLikes = makeLikeList(rows)   
        toCheck = copy.deepcopy(userLikes[1:])  

        if len(toCheck) < 1000:
            #users with more than 1000 take up much more resources/time and are of less          
            #analytical value
            for like in toCheck:



                if len(like) > 30 or len(like) == 0:
                    #This changes the filter sensitivity.  Most useful likes are under 
                    #30 char long
                    userLikes.remove(like)
                else:
                    letCheck = 0
                    for letter in like[:5]:
                        if letter in alphanum:
                           letCheck += 1
                    if letCheck >= len(like[:5])-1:
                        pass
                    else:
                        userLikes.remove(like)

            if len(userLikes) > 1 and len(userLikes[0]) == 32:
                #filters out users with no likes

                scrubbedToCheck = copy.deepcopy(userLikes[1:])
                for like in scrubbedToCheck:
                    if like == 'Facebook' or like == 'YouTube':
                        #youtube and facebook are very common likes but aren't very 
                        #useful
                        userLikes.remove(like)
                    #removes duplicate likes
                    elif like not in repeatCheck:
                        repeatCheck.append(like)
                    else:
                        userLikes.remove(like)



                scrubbedrows = '"'+'","'.join(userLikes)+'"\n'

                if userCount%fileSize == 1:
                    #This block allows for data to be parsed into multiple smaller 
                    #files
                    fileNum += 1
                    dct.append(fileNum)
                    dct[fileNum] = open(fileWritePath + str(fileNum) +'.csv', 'w')
                    if fileNum != 1:
                        dct[fileNum-1].close()
                dct[fileNum].writelines(scrubbedrows)    

            if userCounter(userCount, 'Users Scrubbed:', 200000):
                break

    print 'Total Users Scrubbed:', userCount
    dct[fileNum].close()






def makeLikeList(rowsCSV):
    '''
    changes csv format list into python list, row by row.
    '''    
    userLikes = rowsCSV.strip().split('","')
    #userLikes is now a list of likes instead of one long string
    cells = range(len(userLikes))
    for i in cells:
        userLikes[i] = userLikes[i].strip('"')
    return userLikes


def userCounter(userCount, msg, upLimit):
    '''
    counts number of uses processed
    '''
    if userCount%5000 == 0:
        print msg, userCount
        #shows number of cleaned users thus far, sets stop point if you don't want to 
        #do entire dataset.
    if userCount == upLimit:
        #sets upperbound on amount of data to be processed
        return True






def likeAggregate(likeDir, likeA = ''):
    '''
    Aggregates likes from multiple csv files
    if likeA specified it only passes user with likeA
    '''
    import collections
    import os
    userCount = 0
    cleanLikesList = []
    fileCount = os.listdir(likeDir)

    for i in fileCount:   
        cleanLikesCSV = open(likeDir +'/' + i )

        for rows in cleanLikesCSV: 
            userCount +=1
            userLikes = makeLikeList(rows)                 
            if likeA != '':    
                if likeA in userLikes:
                    cleanLikesList += userLikes[1:]
            else:
                 cleanLikesList += userLikes[1:]

    likeCount = collections.Counter(cleanLikesList)

    return likeCount




def likeUpdate(likeCount, likesToAddCSV):
    '''
    this method is used to update existing like counts if more data is added later on
    it can be used instead of likeAggregate to add to existing data
    instead of recounting everything
    '''
    import collections
    likesToAdd = []
    newLikesCSV = open(likesToAddCSV)

    for rows in newLikesCSV:
        userLikes = makeLikeList(rows)
        likesToAdd += userLikes[1:]

    addCount = collections.Counter(likesToAdd)
    likeCount.update(addCount)
    print 'UPDATED'
    return likeCount




def makeXY(likeDir, like):
    '''
    estimated run time for 170k users: 1min
    this method preps the like counts for modeling
    '''
    import matplotlib_venn
    from matplotlib import pyplot as plt    
    topLikes = likeAggregate(likeDir, like).most_common(20)
    for i in topLikes:
        print i[0]+',', i[1]

    firstPair = likeAggregate(likeDir, topLikes[1][0]).most_common(1)
    Ab = (topLikes[0][1]-topLikes[1][1])
    aB = (firstPair[0][1] - topLikes[1][1])
    AB = (topLikes[1][1]) 
    percent = int(float(topLikes[1][1])/float(topLikes[0][1])*100)
    #title = str(like+' shares '+str(percent)+'% users with ' + topLikes[1][0])
    title = str(str(percent)+'% '+like+' users like '+topLikes[1][0])
    plt.title(title)
    matplotlib_venn.venn2([Ab,aB,AB],(topLikes[0][0],topLikes[1][0]))
    plt.show()




def recommendLikes(likeDir, inputLikes = ''):
    '''
    Estimated runtime for 170k users: 1min
    Prompts user to provide csv list of likes
    Recommends 4 things by normalizing the like-count of the top 10 likes
    then summing the normalized count and selecting the biggest 3 sums.
    Input format is csv: What do you like? Kanye West,Lil Wayne,Eminem

    What do you like? Kanye West,Avril Lavigne,Lil Wayne
    You might also like:
    (Eminem, Rihanna, Drake, Family Guy)
    '''
    import collections
    normCountDict = {}
    recLikes = []
    if inputLikes == '':

        newLikes = raw_input("What do you like? ")
        print 'You might also like:'
        listLikes = newLikes.split(',')
    else:
        listLikes = inputLikes.split(',')

    for like in listLikes:

        pairs = likeAggregate(likeDir, like).most_common(10)

        for i in pairs:
            if i[0] not in listLikes:

                if i[0] in normCountDict:
                    normCountDict[i[0]] += float(i[1])/float(pairs[0][1])
                else:
                    normCountDict[i[0]] = float(i[1])/float(pairs[0][1])
    recommendations = collections.Counter(normCountDict).most_common(4)
    for i in recommendations:
        recLikes.append(i[0])
    if inputLikes == '':
        print '(' + ', '.join(recLikes) +')'
    return recLikes




def recommendUser(likeDir):
    '''
    estimated runtime for 170k users: 2min
    this method prompts user for a csv (no spaces) list of likes.
    It then suggests a list of 4 likes using the recommendLikes method
    using the given likes and suggested likes it finds other users that have similar 
    interests
    Mr. Exact will match all given interests
    Mrs. Close will have at least 1 matching given interest and the rest will be 
    suggested 
    interests
    Ms. Kind-of-Close will have only suggested interests


    What do you like? Eminem,Jay-Z,Tool
    Suggested Likes: (Family Guy, Lil Wayne, Rihanna, Michael Jackson)
    Suggested friends:
    Mr. Exact: 3bbffaa89c146de9ceed944074f047e5 (Eminem, Jay-Z, Tool)
    Mrs. Close: 0f366ec62ad88d34b2190419bd9b22de (Eminem, Jay-Z, Family Guy)
    Ms. Kind-of-Close: 8455b1d0a0c0cfe82984ce3a42b10c12 (Family Guy, Lil Wayne, 
    Rihanna)
    '''
    import collections
    import os
    import random
    friendList = []
    #friendList lists possible friends (just userID) that match some/all of your likes
    maybeFriendList =[]
    #maybeFriendList lists possible friends (just userID) that match your suggested 
    likes
    userListLikes = []
    #userListLikes is friendList but with userID AND likes
    suggestedUserListLikes = []
    #maybeFriendList but with userID AND likes
    exactList =[]
    #exactList is a list of exact matches (userID only)
    exactLikeList = []
    #exactLikeList shows the matching likes shared by Mr. Exact (should match input 
    likes)
    closeList = []
    #closeList is a list of possible friends (just userID) with some like matches and 
    #some 
    #suggested like matches
    closeLikeList = []
    #closeLikeList is list of matched likes and matched suggested likes in Mrs. Close
    kindaCloseList = []
    #kindaCloseList is a list of possible friends (userID only) who match ONLY 
    #suggested 
    #likes
    kindaCloseLikeList =[]
    #kindaCloseLikeList is a list of matched suggested likes in Ms. Kind-of-Close
    newLikes = raw_input('What do you like? ')
    listLikes = newLikes.split(',')
    exactNum = len(listLikes)
    recLikes = recommendLikes(likeDir, newLikes)
    print 'Suggested Likes: (' + ', '.join(recLikes) +')'
    print 'Suggested friends:'
    fileCount = os.listdir(likeDir)

    for i in fileCount:   
        cleanLikesCSV = open(likeDir +'/' + i )
        for rows in cleanLikesCSV:
            friendLikes = makeLikeList(rows)
            for like in listLikes:
                if like in friendLikes: 
                    friendList.append(friendLikes[0])
                    if friendLikes not in userListLikes:
                        userListLikes.append(friendLikes)
            for suggestion in recLikes:
                if suggestion in friendLikes:
                    maybeFriendList.append(friendLikes[0])
                    if friendLikes not in suggestedUserListLikes:
                        suggestedUserListLikes.append(friendLikes)

    bestMatch = collections.Counter(friendList)
    closeMatch = collections.Counter(maybeFriendList)

    for user in bestMatch:
        if bestMatch[user] == exactNum:
            exactList.append(user)
        elif closeMatch[user] >= (exactNum - bestMatch[user]):
                closeList.append(user)

    for user in closeMatch:
        if bestMatch[user] == 0 and closeMatch[user] >= exactNum:
            kindaCloseList.append(user)

    exact = random.choice(exactList)
    close = random.choice(closeList)
    kinda = random.choice(kindaCloseList)
    likeCombo = listLikes + recLikes      

    for user in userListLikes:
        if exact in user:
            for like in listLikes:
                if like in user:
                    exactLikeList.append(like)
            print 'Mr. Exact:', user[0], '(' + ', '.join(exactLikeList) +')'

    for user in userListLikes:
        if close in user:
            for like in likeCombo:
                if like in user and len(closeLikeList) < 3:
                    closeLikeList.append(like)             
            print 'Mrs. Close:',user[0], '(' + ', '.join(closeLikeList) +')'

    for user in suggestedUserListLikes:  
        if kinda in user:
            for like in likeCombo:
                if like in user and len(kindaCloseLikeList) < 3:
                    kindaCloseLikeList.append(like)
            print 'Ms. Kind-of-Close:',user[0], \
                  '(' + ', '.join(kindaCloseLikeList) +')'






def makeUserVector(likeDir):
    '''
    this method strips users of all likes that are not in the top 200
    the purpose of this is to create a standardized user vector for each user
    each like represents a new dimension
    '''
    import os
    import copy
    dimensions = []
    userVectors = []
    likeCount = likeAggregate(likeDir).most_common(200)
    fileCount = os.listdir(likeDir)
    for like in likeCount:
        dimensions.append(like[0])
    for i in fileCount:   
        cleanLikesCSV = open(likeDir +'/' + i )
        for rows in cleanLikesCSV:
            userLikes = makeLikeList(rows)
            toCheck = copy.deepcopy(userLikes[1:])
            for like in toCheck:
                if like not in dimensions:
                    userLikes.remove(like)
            if len(userLikes) > 10:
                userVectors.append(userLikes)

    return userVectors



def findCluster(likeDir, threshold):
    '''
    estimated runtime for 30k users: 1hr and 40min. nearest neighbor algorithm is 
    O(n^2)
    once vectors are created for each user we can calculate the distance between two 
    users
    by setting a threshold for the allowed distance we should see vector clusters begin  
    to emerge
    it starts off very slow but picks up speed toward the end as it remembers past 
    calculations and filters out more and more users
    each seed (user vector) will attempt to create a cluser around it
    as the method iterates through successive vectors in the same cluster it will 
    recreate 
    that particular cluster multiple times
    thus a follow up method must be used to eliminate repeat clusters
    '''
    import numpy
    import copy
    userVectors = makeUserVector(likeDir)
    sampleSize = len(userVectors)
    print 'number of users',sampleSize    
    cluster = {}
    clusterNum = 0

    skipList = {}

    rawVectors = open('filepath'+'rawVectors.csv', 'w')
    rawClusters = open('filepath'+'rawClusters.csv', 'w')
    for vector in range(sampleSize):
        skipList[userVectors[vector][0]] = 0

    for vectorA in range(sampleSize):    
        clusterNum += 1
        checked = 0
        seed = userVectors[vectorA][0]
        cluster[seed] = {'SEED'+seed:0}
        for vectorB in range(sampleSize):               
            leaf = userVectors[vectorB][0]
            if skipList[leaf] == 0:
                checked += 1
                if vectorB <= vectorA:
                    if seed in cluster[leaf]:
                        cluster[seed][leaf] = 1
                else:
                    vecAleftover = copy.deepcopy(userVectors[vectorA][1:])
                    diffVect = []
                    for direction in userVectors[vectorB][1:]:
                        if direction not in userVectors[vectorA][1:]:
                            diffVect.append(direction)
                        else:
                            vecAleftover.remove(direction)
                    diffVect += vecAleftover
                    diffVectMag = numpy.sqrt(len(diffVect))

                    if diffVectMag <= threshold:
                        cluster[seed][leaf] = 1


        if len(cluster[seed]) < 10:
            skipList[seed] = 1

        elif len(cluster[seed]) > 150:
            clusterSize = len(cluster[seed])
            saveVector = [str(clusterSize)]+userVectors[vectorA]
            newVector = '"'+'","'.join(saveVector)+'"\n'
            newCluster = '"'+'","'.join(cluster[seed])+'"\n'
            print 'cluster size:',clusterSize
            #print newVector
            #print newCluster
            print 'CHECKED', checked, 'SKIPPED',sum(skipList.values())
            rawVectors.writelines(newVector)
            rawClusters.writelines(newCluster)
        if clusterNum%50 == 0:
            print 'clusters tried:', clusterNum

    rawVectors.close()
    rawClusters.close()





def uniqueCluster(rawVectors, rawClusters, threshold):
    '''
    estimated runtime: 1min
    this method takes in the raw cluster data provided by findCluster and removed 
    duplicate clusters.
    after cluster list has been cleaned it produces a venn diagram of the top 3 
    clusters and their users
    '''
    import copy
    import numpy
    import matplotlib_venn
    from matplotlib import pyplot as plt
    vectorCSV = open(rawVectors)
    clusterCSV = open(rawClusters)   
    vectorList = []
    repeatList = []
    clusterList = []
    notUnique = []
    topClusters = []
    for rows in vectorCSV:
        vectorList.append(makeLikeList(rows))
    for rows in clusterCSV:
        clusterList.append(makeLikeList(rows))

    for vector in vectorList:
        if vector not in repeatList:
            repeatList.append(vector)
        else:
            vectorList.remove(vector)

    for vectorA in vectorList:
        for vectorB in vectorList:
            vecAleftover = copy.deepcopy(vectorA[1:])
            diffVect = []
            for direction in vectorB[1:]:
                if direction not in vectorA[1:]:
                    diffVect.append(direction)
                else:
                    vecAleftover.remove(direction)
            diffVect += vecAleftover
            diffVectMag = numpy.sqrt(len(diffVect))

            if diffVectMag <= threshold:
                if int(vectorA[0]) > int(vectorB[0]):
                    if vectorB not in notUnique:
                        notUnique.append(vectorB)
                elif int(vectorA[0]) < int(vectorB[0]):
                    if vectorA not in notUnique:
                        notUnique.append(vectorA)

    for vector in notUnique:
        vectorList.remove(vector)

    for vector in vectorList:
        for cluster in clusterList:
            if 'SEED'+vector[1] in cluster:
                 topClusters.append(cluster)



    set1 = set(topClusters[0])
    set2 = set(topClusters[1])
    set3 = set(topClusters[2])

    for vector in range(3):

        print 'Cluster' + str(vector+1)+'-'+vectorList[vector][1]+ \
              ':(' + ', '.join(vectorList[vector][2:]) +')'



    title = 'Nearest-Neighbor Clusters'
    plt.title(title)
    matplotlib_venn.venn3([set1, set2, set3],('Cluster1','Cluster2','Cluster3'))
    #matplotlib_venn.venn3([set1, set2, set3],(vectorList[0][1],vectorList[1]
    #[1],vectorList[2][1]))
    plt.show()


#cleaner(likes)
#likeCount = likeAggregate(likeDir)
#likeUpdate(likeCount, likesToAddCSV)
#makeXY(likeDir, 'Jay-Z')  
#recommendLikes(likeDir)
#recommendUser(likeDir)
#findCluster(newLikeDir, 4)
#uniqueCluster(rawVectors, rawClusters, 4.4)
\$\endgroup\$
1
  • 1
    \$\begingroup\$ @TobySpeight I think this question is OK; it's just that the description is hidden under the cleaner method. (I fixed the title) \$\endgroup\$
    – t3chb0t
    Commented Sep 4, 2019 at 9:18

3 Answers 3

3
\$\begingroup\$

Making your code run

Here's the result of my first run :

  File "cleaner.py", line 305
    likes)
         ^
SyntaxError: invalid syntax

This is quite easy to fix : just comment this part of the line or put it with the part of the code it goes with.

Here's the result of the second run :

Traceback (most recent call last):
  File "cleaner.py", line 19, in <module>
    likes = open('filepath' + 'likes.csv')
IOError: [Errno 2] No such file or directory: 'filepathlikes.csv'

Fixed by commenting the line likes = open('filepath' + 'likes.csv') (and moving it to the bottom of the file where it belongs).

Style

There is a common Python style guide described in PEP 8.

You can find a few tools that will help you to spot a few things than can easily be improved :

I'll let you play with this because there is quite a lot to say :

Documentation

Your code needs some king of comments/docstring so that one can understand what it is all about.

Focusing on smaller pieces of code

As I have no idea what the big pictures is, I'll give you some tips on smaller parts of the code that can easily be improved :

You can rewrite :

                letCheck = 0
                for letter in like[:5]:
                    if letter in alphanum:
                       letCheck += 1

with the sum function :

                letCheck = sum(1 for letter in like[:5] if letter in alphanum)

You can rewrite

                if letCheck >= len(like[:5])-1:
                    pass
                else:
                    userLikes.remove(like)

by inverting the condition :

                if letCheck < len(like[:5])-1:
                    userLikes.remove(like)

You can rewrite :

cells = range(len(userLikes))
for i in cells:
    userLikes[i] = userLikes[i].strip('"')
return userLikes

by using list comprehension :

return [l.strip('"') for l in userLikes]

Your userCounter, on top on having a pretty bad name does not always return something. It probably be a good idea to add a return False at the end to make it better but if you are to write this :

if userCount == upLimit:
    #sets upperbound on amount of data to be processed
    return True
return False

You should probably do this instead :

return userCount == upLimit

You can rewrite :

        if likeA != '':
            if likeA in userLikes:
                cleanLikesList += userLikes[1:]
        else:
             cleanLikesList += userLikes[1:]

by inverting the conditions :

        if likeA == '':
             cleanLikesList += userLikes[1:]
        else:
            if likeA in userLikes:
                cleanLikesList += userLikes[1:]

merging else and if

        if likeA == '':
             cleanLikesList += userLikes[1:]
        elif likeA in userLikes:
             cleanLikesList += userLikes[1:]

and then merging conditions :

        if likeA == '' or likeA in userLikes:
             cleanLikesList += userLikes[1:]

Also, in the same function, the userCount variable does not seem to be used.

There is still quite a lot to say but I am running out of time. As a general rule of thumb, you should try to keep your function smaller and to make it clearer what the intent is.

\$\endgroup\$
0
3
\$\begingroup\$

I don't think Josay mentioned these yet.


for rows in likes: 
    repeatCheck = []        
    userCount += 1

Since you are counting what you are iterating, it would be Pythonic to use enumerate:

for userCount, rows in enumerate(likes, start=1):

Also, repeatCheck above is only used for in and appending, so a set would be appropriate.


You have several instances of:

copy.deepcopy(userLikes[1:])

For no good reason. Slicing userLikes[1:] already creates a new list. You don't seem to be modifying anything you take out of the list either, which would explain a deep copy. Even worse, you then use the likes in:

userLikes.remove(like)

which could be wrong for some deepcopied objects (it's not clear what object like is and is composed of).

If there is a reason for the deep copy that I'm not seeing, it should definitely be mentioned in a comment. Right now it just looks like defensive programming gone wrong.


One of my pet peeves is:

for rows in likes: 
    #...
    if len(toCheck) < 1000:
        # huge if block containing the rest of the loop

If you are going to have a skip path, you can use continue to avoid indentation. This often makes the logic clearer as well.

for rows in likes: 
    #...
    if len(toCheck) >= too_large:
        continue

Now the code clearly says "skip too large values". Note naming the magic value – not always necessary, but can lead to self documenting code.

(Same is possible with return in functions. It doesn't always lead to clearer code, so it's not a pattern to blindly follow.)

\$\endgroup\$
4
  • \$\begingroup\$ Hi Otus, thanks for answering. The reason I used deepcopy was that there is a list I am iterating through, elements from this list are removed if they meet certain conditions. If I remove items from a list I am currently iterating through it will cause the loop to skip or miscount. My logic here was if I do not use deepcopy and start changing the list it will also change the list I am iterating through. Does that make sense? \$\endgroup\$
    – Ralph
    Commented Jun 6, 2014 at 8:22
  • \$\begingroup\$ @Ralph: That's a good reason to make a copy, but does not require a deep copy. Also, like I mentioned, userLikes[1:] already creates a copy of the list. \$\endgroup\$
    – otus
    Commented Jun 6, 2014 at 8:32
  • \$\begingroup\$ Ah ok, I didn't realize that. Any clue what they meant by "JavaScript style string interpolation"? I have never used JavaScript so I don't understand what this means or why it's bad. \$\endgroup\$
    – Ralph
    Commented Jun 6, 2014 at 8:51
  • \$\begingroup\$ @Ralph, possibly the frequent use of 'a' + variable + 'b' and str(variable) rather than the formatting operator/function: 'a%sb' % variable or 'a{var}b'.format(var=variable). \$\endgroup\$
    – otus
    Commented Jun 6, 2014 at 9:09
3
\$\begingroup\$

I have some suggestions to point out:

  1. import statements should always go at the top. This saves from accidentally importing the same module twice in two separate sections of code; which you actually do. You import collections in your: likeUpdate function, and your recommendLikes function, your recommendUser function.
  2. When creating filepaths, use os.path.join. This uses the OS-specific separator:

    cleanLikesCSV = open(os.path.join(likeDir, i))
    
  3. Pythonic variable names use the underscores_in_names style instead of camelCase. The same style applies to function/method names. The only aberration to this rule is class names which use PascalCase.

  4. Use desriptive variable names. What is Ab? What about aB? AB? Those 3 examples were used one after the other in your makeXY function. It is very difficult (if not impossible in this case) to understand what those variables are supposed to do and hold.

  5. This statement:

    Ab = (topLikes[0][1]-topLikes[1][1])
    

    contains extraneous parentheses. You can remove them and not change the logic of the program.

  6. Whitespace is your friend. Separate logical blocks of code with a single blank line. However, be careful you don't use TOO much whitespace. In your code you typically have 3-4 blank lines separating fuctions. This is too much; a single line will do.

  7. You populate lists by first declaring them and then populating them using for loops. This can be compacted down to a single list comprehension:

    # This is your current implementation
    vectorList = []
    for rows in vectorCSV:
        vectorList.append(makeLikeList(rows))
    
    # This is how you SHOULD do it
    vector_list = [makeLikeList(rows) for rows in vectorCSV]
    

My final point, which I chose to separate from the above list because of its importance, is: always use with syntax when dealing with files.

Currently, you use the open() syntax. This is technically correct. However, it brings with it a slew of possible places to introduce bugs into your code. The biggest bug is when you forget to call close() on the file.

You do this A LOT. You called the open() function 12 separate times while you only called the close() function 4 times. This means that you have open file pointers to at least 8 files when your code finishes.

To skip this bug, use the with syntax. The with keyword automatically closes any context manager (file object, etc.) once its block finishes. An example:

with open('some_file.txt', 'r') as file:
    for line in file:
        print(line)

# Once the program gets here, file is now closed.
foo = 'Hello World!'
\$\endgroup\$
2
  • \$\begingroup\$ @DariunDouglas thanks for the suggestions, very helpful. I was wondering with the os.path.join part, why is this better? it looks like it just adds something extra to the code? \$\endgroup\$
    – Ralph
    Commented Jun 9, 2014 at 23:02
  • \$\begingroup\$ Its better because it uses the system specific slashes. So on Windows systems it will use \ while on basically all other systems it will use /. \$\endgroup\$ Commented Jun 10, 2014 at 11:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.