Collate votes on MP3 files to list them by popularity

Question

For my first Python project, I decided to create a script that would automatically collate MP3 files into a music countdown. I wanted to 'learn by doing', so I dived straight in and used the Python docs and Stack Overflow as my primary learning resource.

The idea is as follows:

• Python script resides in base directory.
• In the base directory there are sub-folders for each voter containing their MP3 files. Each sub-folder is named with the voter's name.
• Each voter submits their MP3 files in the following format:

(vote_count). Artist - Song Title.mp3.

So, for example, 10. Foo Fighters - Walk.mp3. The '10.' at the start of the song specifies how many votes are being allocated to this song.

• Once sub-folders have been populated and named correctly in the format, the script can be executed.
• Upon execution, the script will scan all sub-folders for files ending in .mp3
• It will then strip the necessary information such as vote count and song title and add the songs to a list.
• Any duplicates (i.e multivotes) and the script will add the votes together to get the total vote count for that song.
• Final results will then output to a CSV file in order of song with highest votes first.

So far so good. I have completed everything in that list, however there are still some more features i would like to add, like renaming all of the MP3 files so they are in the correct order for importing to a playlist. I.e add numbers to the start of each filename.

import os, csv, operator, itertools
from collections import Counter, defaultdict

print("*****Welcome to the Music vote Collator*****" + '\n')
print("STAGE 1. Processing Votes..." + '\n')
folder_list = os.getcwd() #Get list of files in all directories

#global variables and lists
song_list = []
voter_list = defaultdict(list)
tally = Counter()
total_songs = 0

for subdir, dirs, files in os.walk(folder_list):
    for file in files:
        if file.endswith(".mp3"):
            base_dir = os.path.join(subdir) #defines base directory...maybe not needed anymore
            file_name = file.split('.')[1] #removes the .mp3 extension and numbers at the start of the file to clean up song name
            vote_count = file.split('.')[0] #retrieves vote count for each song
            head, tail = os.path.split(subdir) #retrieves the name of the voter from the folder name
            song = file_name.lower() #create variable for song and convert all letters to lowercase to prevent song name match issues
            if (song not in song_list and
                song not in tally):
                song_list.append(song) #appends song to song_list to keep track of processed songs

            if (song in song_list and
                song not in tally):
                tally[song] += 0 #for duplicate entries, adds the song to the tally Counter
                total_songs += 1 #adds plus one to the total_songs variable for stat reporting

            if (song in song_list and
                song in tally):
                tally[song] += int(vote_count) #for duplicate entries, adds the votes to the song in tally Counter
                voter_list[song].append(tail)
                voter_list[song].append(vote_count)

tally_list = sorted(list(Counter((tally)).items()),key=operator.itemgetter(1), reverse=True) #converts tally counter to a list and sorts songs based on highest votes
with open('resultsout.csv', 'w') as resultsout:
    writer = csv.writer(resultsout)
    writer.writerows(tally_list)

print("STAGE 2. Listing Top 3 Songs:" + '\n')
with open('resultsout.csv', 'r') as results:
    reader = csv.reader(results)
    for row in itertools.islice(reader, 0, 1):
        print("The number one song for this year's count is:" + str(row[0]) + ", with a total of " + str(row[1]) + " votes!")
    for row in itertools.islice(reader, 0, 1):
        print("The number two song for this year's count is:" + str(row[0]) + ", with a total of " + str(row[1]) + " votes!")
    for row in itertools.islice(reader, 0, 1):
        print("The number three song for this year's count is:" + str(row[0]) + ", with a total of " + str(row[1]) + " votes!")
    print('\n' + "There are a total number of " + str(total_songs) + " songs in this year's count.")

I understand that I should have defined functions. I only just discovered it. The code works fine anyway as I have no real need to call functions; everything just happens in order. Is there any reason to use functions in this script?

Something I haven't figured out yet is how to append the voter's name and votes to each song they vote for. The counter does support append.

I would appreciate anyone who takes the time to provide feedback/recommendations. I'm sure someone else here could probably do the same thing in about 10 lines, but bear in mind this is all a learning experience.

Answer 1

Avoid wrong comments

folder_list = os.getcwd() #Get list of files in all directories

So the comment states that os.getcwd() -> List[Filenames] but the docs say:

>>> help(os.getcwd)
Help on built-in function getcwd in module posix:

getcwd(...)
    getcwd() -> path

    Return a unicode string representing the current working directory.

A comment that says that the code does another thing from what it actually does is incredibly confusing.

Avoid obvious comments

Some comments may be needed, but stating the obvious in comments is frowned upon.

For example:

song = file_name.lower() #create variable for song and convert all letters to lowercase to prevent song name match issues

If you see x = you know it is a variable assignment, and .lower() obviously converts to lower case.

The only valuable part is to prevent song name match issues because it tells me why you are doing what you can see are doing from the code so I would change that to:

song = file_name.lower() # Lower-casing prevents song-name match issues.

Instead #global variables and lists can be deleted wholesale as it adds no information at all to the code.

Unpacking

A neat trick where you can replace

a = lst[0]
b = lst[1]

with

a, b = lst

In practice:

    file_name = file.split('.')[1] #removes the .mp3 extension and numbers at the start of the file to clean up song name
    vote_count = file.split('.')[0] #retrieves vote count for each song

Becomes:

vote_count, file_name = file.split('.')

Simpler for loops

for row in itertools.islice(reader, 0, 1):
    print("The number one song for this year's count is:" + str(row[0]) + ", with a total of " + str(row[1]) + " votes!")
for row in itertools.islice(reader, 0, 1):
    print("The number two song for this year's count is:" + str(row[0]) + ", with a total of " + str(row[1]) + " votes!")
for row in itertools.islice(reader, 0, 1):
    print("The number three song for this year's count is:" + str(row[0]) + ", with a total of " + str(row[1]) + " votes!")

itertools.islice(xs, 0, 1) just returns the first item of xs, doing it three times equals getting the first three items, so you can greatly simplify:

for iteration, row in enumerate(csv.reader(results)):
    print("The number {} song for this year's count is:{}, with a total of {} votes!".format(
        ["one", "two", "three"][iteration], row[0], row[1]))
    if iteration >= 3:
        break

Note:

• The use of .format instead of + with strings, as the former is more flexible and readable.

• The use of a loop repeat 3 times, instead of three identical loops repeat 3 times.

• The simplicity in raising the number of songs to print, just change 3 to something else along with the ["one", "two", "three"] list.

• The use of enumerate, that gives the index of the item along with the item. At times it is much more readable than indexing the collection (indexing may also not always be possible)

Just delete dead code

dead code is code that serves no purpose, and just sits there making the program harder to read. I am talking about base_dir = os.path.join(subdir) You yourself state:

maybe not needed anymore

I can argue, surely not needed anymore, just use Control-F and look for base_dir and you will find no uses at all! Just delete it and enjoy the simpler code.

Follow the underscore convention

You write:

head, tail = os.path.split(subdir) #retrieves the name of the voter from the folder name

But head is never used again, the Pythonic way of saying I don't care about that value is giving it the _ [UNDERSCORE] name, this way readers will stop looking all over the code for potential uses of head. All in all, using _ is a variable less to keep track of, one further step towards code simplification.

Answer 2

In addition to good points by Caridorc, there are three simple points I would like to make:

• Remove total_songs – The code related to counting the total_songs whilst building the lists only complicates your code. This count is easily found afterwards using len(song_list) if I'm not mistaken, or if removing song_list by counting keys in tally.

• Simplify the if-statements related to song in song_list and/or tally – First of all you don't seem to use song_list for anything but keeping track of presences, but this can be done checking for existence of key in tally. Secondly you add 0 to the tally, which seems strange. Finally you could join the voter and vote as a tuple in the voter_list. So the following untested code should basically be enough:

      #for duplicate entries, adds the votes to the song in tally Counter
      tally[song] += int(vote_count)
  
      # Keep track of who voted what
      voter_list[song].append((tail, int(vote_count)))
  

  You could possibly subtract the int(vote_count) to avoid doing it twice.

• Avoid top level code, and use functions. :-) – You've commented upon it, and I would just like to emphasize that using functions is something good which you should look into as soon as possible. In this case it could be argued how many functions you need to create, but I would at least create one function, named main() or collate_votes(). Your code is almost neatly divided into functions already, so I would aim for main() looking something like:

  def main():
      filename = 'resultsout.csv'
      tally, voter_list = collate_votes(os.getcwd)
      write_tally(tally, filename)
      process_result(filename)
      # print_voter_list(voter_list)
  
  if __name__ == '__main__':
      main()
  

  Doing this would allow for using your script as a module by other scripts, building extension which could calculate votes from different directories, do other processing handling of voter list and/or tally. And still due to the use of if __name__ == '__main__' if you call it directly as the script, the basic functionality is preserved.

Answer 3

An addition to @Caridorc answer:

If your code works correctly (I hope it is), there is a simplification to be done. Let me first explain why I doubt:

# assume song is not in song_list and not in tally, so we go inside this "if"
if (song not in song_list and
    song not in tally):
    song_list.append(song)
    # now song is in song_list

# if it was elif, we would skip this, but it is "if", so in we go
if (song in song_list and
    song not in tally):
    tally[song] += 0 
    total_songs += 1 
    # and now song is in tally

# and this also evaluates to True now. Go inside again
if (song in song_list and
    song in tally):
    tally[song] += int(vote_count) #for duplicate entries, adds the votes to the song in tally Counter
    voter_list[song].append(tail)
    voter_list[song].append(vote_count)

Now let's assume it is correct. You make lots of checks which aren't necessary. You can simplify it to

# looks like you never use the song_list. Better to remove this if so.
if song not in song_list:
    song_list.append(song)

if song not in tally:
    # btw, shouldn't it be just '='?
    # and is this line really needed?
    # You will add the score later, so why adding 0 first?
    tally[song] += 0
    total_songs += 1

# if you want to be fool-proof, use try except here (I'm sure you will find docs yourself), or check if vote_count.isdigit()
tally[song] += int(vote_count)
# Do you ever use the voter_list? If not, then just remove it
# and I'm not sure which is better, but I would rather do it like this:
voter_list[song] + [tail,vote_count]