Find and remove duplicate files in one or more directories

Question

I needed this to clean up a couple of similar music libraries, which also contained some empty files and duplicate files with different names.

# dupFinder.py
import os, sys, shutil, datetime
import hashlib

def findDup(parentFolder):
    # Dups in format {hash:[names]}
    dups = {}
    for dirName, subdirs, fileList in os.walk(parentFolder):
        print('Scanning %s...' % dirName)
        for filename in fileList:
            # Get the path to the file
            path = os.path.join(dirName, filename)
            print(path)
            # Calculate hash
            file_hash = hashfile(path)
            # Add or append the file path
            if file_hash in dups:
                dups[file_hash].append(path)
            else:
                dups[file_hash] = [path]
    return dups


# Joins two dictionaries
def joinDicts(dict1, dict2):
    for key in dict2.keys():
        if key in dict1:
            dict1[key] = dict1[key] + dict2[key]
        else:
            dict1[key] = dict2[key]


def hashfile(path, blocksize = 65536):
    print(path)
    afile = open(path, 'rb')
    hasher = hashlib.md5()
    buf = afile.read(blocksize)
    while len(buf) > 0:
        hasher.update(buf)
        buf = afile.read(blocksize)
    afile.close()
    return hasher.hexdigest()


def handleResults(dict1):
    results = list(filter(lambda x: len(x) > 1, dict1.values()))
    if not 'testrun' in globals():
        backupdir = 'removed_' + str(datetime.datetime.now())
        if not os.path.exists(backupdir):
            os.makedirs(backupdir)
    else:
        backupdir = 'backup_dir_date_now'
    if len(results) > 0:
        print('Duplicates Found:')
        print('The following files are identical. The name could differ, but the content is identical')
        print('^^^^^^^^^^^^^^^^^')
        count = 0
        for result in results:
            for subresult in result:
                if count == 0:
                    print('KEEPING ONE COPY: %s.' % subresult)
                    count += 1
            elif 'testrun' in globals():
                print('\t\t %s WILL BE REMOVED TO: %s' % (subresult, backupdir))
            else:
                print('\t\tREMOVING %s TO %s' % (subresult, backupdir))
                try:
                    shutil.move(subresult, backupdir)
                except shutil.Error:
                    print ('\t\t EXISTS. DELETING.')
                    os.remove(subresult)
                print('___________________')

    else:
        print('No duplicate files found.')


if __name__ == '__main__':
    if len(sys.argv) > 1:
        dups = {}
        if sys.argv[1] == '-t':
            testrun = 1
            folders = sys.argv[2:]
        else: folders = sys.argv[1:]
        for i in folders:
            # Iterate the folders given
            if os.path.exists(i):
                # Find the duplicated files and append them to the dups
                joinDicts(dups, findDup(i))
            else:
                print('%s is not a valid path, please verify' % i)
                sys.exit()
        handleResults(dups)
    else:
        print('Usage: python dupFinder.py [-t] folder or python [-t] dupFinder.py folder1 folder2 folder3')

It can be run in test mode with -t flag.

Wow - awesome advice here sure transformed the code: GitHub Repo

Answer 1

• Imports should be separated onto multiple lines, not imported on the same line, as seen in the below example.

---

import import 
import os
import sys
import shutil
import datetime

---

• If you're also going to be using only a couple function/class/variables from a module, you can import them using from ... import ....
• Functions and variables should be in underscore_case, and classes should be in PascalCase.
• You have a lot of code underneath if __name__ == "__main__":. Consider finding a way to separate this code into a main method. if __name__ == "__main__": should usually be about as simple as this.

---

if __name__ == "__main__":
    main( ... )

---

• You have some inconsistent indentation. E.g, some places are indendented by four spaces/a tab, and others, well, more spaces/tabs.
• Rather than putting comments over functions, use docstrings to describe the purpose of a function, like this.

---

def my_function(args):
    """
    Describe the purpose of your function
    here and it's arguments.
    """
    ...

---

• My final tip would be to read PEP8, Python's official style guide, and see how you can properly format your code.

If there's anything else that you want me to cover, just mention it in the comments, and I'll see what I can do.

Answer 2

If I had to pick a least-favourite line in the program, it would be this one:

if 'testrun' in globals():

… because

• Global variables are bad.
• You didn't even declare testrun as a global variable using global testrun, but rather you used introspection.
• You didn't test the flag to see whether it was True or False; you just check for its existence.
• The whole problem could have been avoided by calling handleResults() with a parameter.

---

print('%s is not a valid path, please verify' % i)
sys.exit()

When you exit the program due to an error, exit with a non-zero status.

i is an unconventional iteration variable, since it has the connotation of being an integer. The code would be more readable if you had written for folder in folders: ….

---

In hashfile(), you should call open() using a with block.

Since hashfile() seems like it could be a basic reusable operation, I would avoid having it print anything. The findDup() function, which calls hashfile(), prints status updates, and in fact it prints the same path just before calling hashfile().

Answer 3

Just reviewing the hashfile function.

1. There's no docstring. What does this function do?

2. The MD5 hash is not collision-resistant: this means that an attacker can create two files with the same hash. This could be disastrous if hashfile were used for data integrity or content de-duplication. Use SHA-256 instead.

3. The condition len(buf) > 0 can be written more simply as buf.

4. The duplicated line of code here:

   buf = afile.read(blocksize)
   while len(buf) > 0:
       hasher.update(buf)
       buf = afile.read(blocksize)
   

   can be avoided if you put the loop termination condition in the middle, like this:

   while True:
       buf = afile.read(blocksize)
       if not buf:
           break
       hasher.update(buf)
   

   This is a standard pattern in Python: see PEP 314.