Find out all the different files from two different paths efficiently in Windows (with Python)

Question

Well, recently I'm working on a program which is able to sync files between different folders.

However, as a fresh man, the algorithm I worte seems feasible, but looks really disgusting, and inefficient as well (bruh~)

The following lines are the related things I wrote,

I really hope there would be experts who can help me improve my code

def scan_items(folder_path):  # 扫描路径下所有文件夹
    def scan_folders_in(f_path):  # 扫描目录下所有的文件夹，并返回路径列表
        surf_items = os.scandir(f_path)
        folders = [f_path]
        for item_data in surf_items:
            if item_data.is_dir():
                folders.extend(scan_folders_in(item_data.path))  # 继续遍历文件夹内文件夹，直到记下全部文件夹路径
        folders = sorted(set(folders))  # 排序 + 排除重复项
        surf_items.close()
        return folders

    file_store = []
    folder_store = scan_folders_in(folder_path)
    for folder in folder_store:  # 遍历所有文件夹
        files = [folder + '\\' + dI for dI in os.listdir(folder) if os.path.isfile(os.path.join(folder, dI))]
        # 如上只生成本文件夹内 文件的路径
        file_store.extend(files)  # 存储上面文件路径
    for i in range(len(file_store)):
        file_store[i] = file_store[i][len(folder_path)::]  # 返回相对位置
    result = [folder_store, file_store]
    return result

And here is the main part, and the 'get_task()' part is the most important one

def sf_sync_dir(path1, path2, single_sync, language_number, area_name=None, pass_item_rpath='', pass_folder_paths=''):
    from LT_Dic import sf_label_text_dic  # Label information from another file

    def sf_show_notice(path_1, path_2, sf_errorname):  # Win10toast used

        toaster.show_toast('Sync Successfully',
                           'The Files in "' + path_1 + '" and "' + path_2 + '" are Synchronized',
                           icon_path=mf_data_path + r'Movefile.ico',
                           duration=10,
                           threaded=False)
        if len(sf_errorname) > 0:
            toaster.show_toast("Couldn't sync files",
                               sf_errorname + sf_label_text_dic['can_not_move_notice'][language_number],
                               icon_path=mf_data_path + r'Movefile.ico',
                               duration=10,
                               threaded=False)

    def get_task(barroot):  # shitty block here, help me!  (baroot is a root showing progress)
        all_files_1 = scan_items(path1)[1]
        all_files_2 = scan_items(path2)[1]
        sync_tasks = []
        pass_folder_rpaths = []
        task_number = 0

        for pass_folder in pass_folder_paths.split(','):
            if pass_folder.startswith(path1):
                pass_folder_rpaths.append(pass_folder.replace(path1, path1.split('\\')[-1]))
            elif pass_folder:
                pass_folder_rpaths.append(pass_folder.replace(path2, path2.split('\\')[-1]))

        file_info_1 = {}  # 存储文件1的信息：(哈希值, 大小, 修改时间)

        for file1 in all_files_1:
            file1_path = path1 + file1
            file_info_1[file1] = (filehash(file1_path), os.path.getsize(file1_path), os.path.getmtime(file1_path))

        for file2 in all_files_2:
            file2_path = path2 + file2

            if file2 in all_files_1:
                file1 = file2
                file1_path = path1 + file1
                file_info = file_info_1[file1]
                file2_info = (filehash(file2_path), os.path.getsize(file2_path), os.path.getmtime(file2_path))

                if file_info == file2_info:
                    continue

                if single_sync and file_info[0] == file2_info[0]:
                    continue

                new_file, old_file = file1, file2
                new_file_path, old_file_path = file1_path, file2_path
                new_file_rpath = path1.split('\\')[-1] + file1
                old_file_rpath = path2.split('\\')[-1] + file2

                if int(os.stat(new_file_path).st_mtime) < int(os.stat(old_file_path).st_mtime):
                    if single_sync:
                        continue
                    old_file, new_file = new_file, old_file
                    new_file_path, old_file_path = old_file_path, new_file_path
                    new_file_rpath, old_file_rpath = old_file_path, new_file_rpath

                if any(pfolder.startswith(old_file_rpath) for pfolder in pass_folder_rpaths) or any(
                        old_file.endswith(pfile) for pfile in pass_item_rpath.split(',')):
                    continue

                task_number += 1
                barroot.set_label1(sf_label_text_dic['main_progress_label'][language_number] + file1.split('\\')[-1])
                sync_tasks.append([new_file_path, old_file_path, False])
            else:
                new_file_rpath = path2.split('\\')[-1] + file2

                if any(pfolder.startswith(new_file_rpath) for pfolder in pass_folder_rpaths):
                    continue

                task_number += 1
                barroot.set_label1(sf_label_text_dic['main_progress_label'][language_number] + file2.split('\\')[-1])
                barroot.progress_root.update_idletasks()
                sync_tasks.append([file2_path, path1 + file2, True])

        if not single_sync:
            for file1 in all_files_1:
                if file1 not in all_files_2:
                    file1_path = path1 + file1
                    newfile1_rpath = path2.split('\\')[-1] + file1

                    if any(pfolder.startswith(newfile1_rpath) for pfolder in pass_folder_rpaths):
                        continue

                    task_number += 1
                    barroot.set_label1(
                        sf_label_text_dic['main_progress_label'][language_number] + file1.split('\\')[-1])
                    barroot.progress_root.update_idletasks()
                    sync_tasks.append([file1_path, path2 + file1, True])

        return sync_tasks

    def synchronize_files(baroot, task):
        baroot.set_label2(sf_label_text_dic["current_file_label1"][language_number] + task[0].split('\\')[-1])
        new_file_path, old_file_path, create_folder = task
        if create_folder:
            try:
                sf_creat_folder(old_file_path)
            except:
                pass
        try:
            shutil.copy2(new_file_path, old_file_path)
        except:
            return new_file_path
        return None

    def run_sync_tasks(baroot):
        sf_errorname = ''
        baroot.main_progress_bar['value'] = 0
        baroot.progress_root.update_idletasks()
        tasks = get_task(baroot)
        baroot.main_progress_bar['maximum'] = len(tasks)
        baroot.set_label1(
            f'{sf_label_text_dic["main_progress_label1"][language_number][0]}{str(baroot.main_progress_bar["value"])}/{str(len(tasks))}  {sf_label_text_dic["main_progress_label1"][language_number][1]}')

        with ThreadPoolExecutor() as executor:
            futures = [executor.submit(synchronize_files, baroot, task) for task in tasks]

            for future in as_completed(futures):
                result = future.result()
                if result:
                    sf_errorname += result + ' , '

                baroot.main_progress_bar['value'] += 1
                baroot.set_label1(
                    f'{sf_label_text_dic["main_progress_label1"][language_number][0]}{str(baroot.main_progress_bar["value"])}/{str(len(tasks))}  {sf_label_text_dic["main_progress_label1"][language_number][1]}')
                baroot.progress_root.update_idletasks()

        baroot.progress_root.withdraw()
        path_name_1 = path1.split('\\')[-1]
        if area_name:
            path_name_1 = area_name
        try:
            sf_show_notice(path_name_1, path2.split('\\')[-1], sf_errorname)
        except:
            pass
        finally:
            baroot.progress_root.withdraw()

    global sync_bar_root, sync_bar_root_task
    sync_bar_root = ProgressBar('Movefile  -Syncfile Progress',
                                sf_label_text_dic["main_progress_label2"][language_number],
                                sf_label_text_dic["current_file_label"][language_number],
                                language_number)
    sync_bar_root_task = threading.Thread(target=lambda: sync_bar_root.launch(), daemon=True)
    sync_bar_root_task.start()
    while not sync_bar_root.initialization_done:
        time.sleep(0.01)
    run_tasks = threading.Thread(target=lambda: run_sync_tasks(sync_bar_root), daemon=True)
    run_tasks.start()

And the progress bar class:

class ProgressBar:
    def __init__(self, title, label1, label2, lang_num):
        self.initialization_done = False
        from LT_Dic import progress_root_label_dic
        self.title = title
        self.label1 = label1
        self.label2 = label2
        self.label_dic = progress_root_label_dic
        self.lang_num = lang_num
        self.main_progress_label = None
        self.main_progress_bar = None
        self.current_file_label = None
        self.show_running_bar = None
        self.progress_root = None
        self.roll_bar = None

    def set_label1(self, content):
        self.main_progress_label['text'] = content

    def set_label2(self, content):
        self.current_file_label['text'] = content

    def launch(self):
        self.progress_root = tk.Tk()
        self.progress_root.title(self.title)
        self.progress_root.geometry('420x115')
        self.progress_root.iconbitmap(mf_data_path + r'Movefile.ico')
        self.main_progress_label = ttk.Label(self.progress_root, text=self.label1)
        self.main_progress_label.grid(row=0, column=0, padx=10, pady=5, sticky='SW')
        self.main_progress_bar = ttk.Progressbar(self.progress_root)
        self.main_progress_bar.grid(row=1, column=0, padx=10, pady=0, ipadx=150, sticky='W')
        self.current_file_label = ttk.Label(self.progress_root, text=self.label2)
        self.current_file_label.grid(row=2, column=0, padx=10, pady=5, sticky='SW')
        self.show_running_bar = ttk.Progressbar(self.progress_root, mode='indeterminate')
        self.show_running_bar.grid(row=3, column=0, padx=10, pady=0, ipadx=150, sticky='W')
        self.progress_root.protocol('WM_DELETE_WINDOW', lambda: self.sync_bar_on_exit())
        self.roll_bar = threading.Thread(target=self.show_running, daemon=True)
        self.roll_bar.start()
        self.initialization_done = True
        self.progress_root.mainloop()

    def show_running(self):
        self.show_running_bar.start(10)

    def sync_bar_on_exit(self):
        if tkinter.messagebox.askyesno(title='Syncfile', message=self.label_dic['confirm_exit_text'][self.lang_num]):
            self.progress_root.withdraw()
            self.roll_bar.join()
            return True
        else:
            return False

    def progress_root_destruction(self):
        self.progress_root.quit()
        self.progress_root.destroy()

And the label part here, if you want~

sf_label_text_dic = {
    'main_progress_label': ['扫描文件中...  发现文件：', 'Scanning items...  Found item：'],
    'main_progress_label1': [['总进度：', '已完成'], ['Progress：', 'Completed']],
    'main_progress_label2': ['扫描文件中...', 'Scanning items...'],
    'current_file_label': ['等待中...', 'Waiting...'],
    'current_file_label1': ['同步中文件：', 'File in process：'],
    'exit_sync': ['''文件正在同步中，
确定中断同步进程并退出?''', '''Synchronization is in progress,
Are you sure to interrupt the process and exit?'''],
    'can_not_move_notice': ["""
无法被移动，请在关闭文件或移除重名文件后重试""", """
Couldn't be moved, Please try again after closing the file
or removing the duplicate file """]
    }

progress_root_label_dic = {
    'confirm_exit_text': ['''文件正在复制中，
确定中断进程并退出?''', '''The file is currently being copied,
Are you sure to interrupt the process and exit?''']
}

It sucks, as you can see......

The main problem is how to improve the efficiency of the get_task() function, because it always takes a long time to find out the files needed to be synchronized.

Probably we don't have to compare the hash of the files at all? Some advice here too.

So, help me write a more efficient code to implement the functions contained in the original code, including single-direction sync option, ability to keep spicific files unchangeable, and keep new files only when syncing, and so on you can see from my original code...

And of course, make the structure of the new code looks more professional if you can~ please~

Tip: you can see the whole program in github: https://github.com/HNRobert/Movefile, if you are interested in the program, you can give me some more suggestions there!

Answer 1

Python libraries

The first problem I see is that there is a lot of code that can be replaced with built-in Python functions.

os.walk()

You can replace the entire scan_items() function with os.walk(). The function walks down through a directory, giving lists of files and folders in each directory found. It is used like this:

for current_directory, folders, files in os.walk(path1):
    # current_directory is the current directory location of the walk
    # folders is a list of directories in the current directory
    # files is a list of the files in the current directory

If you want to control which files and folders are skipped (keeping them unchanged), you can edit the lists in-place. Anything removed from the folders list will be skipped by os.walk(). Use folders[:] = edited_folder_list instead of folders = edited_folder_list. The latter has no effect on os.walk().

filecmp.cmpfiles()

The other library you should look into for get_task() is filecmp, which has a set of functions for comparing files. Specifically, filecmp.cmpfiles() is probably what you want to use instead of hashing (see more comments on that in the next section).

matching, mismatching, errors = filecmp.cmpfiles(path_1, path_2, file_list, shallow=False)

This call will take a list of files (file_list) and compare the versions in path_1 and path_2, sorting the files into three lists:

• matching: files that match according to the shallow criteria.
• mismatching: files that are different.
• errors: files that could not be compared due to errors such as not existing in one of the directories.

The shallow parameter controls whether the whole of both files are examined for differences (shallow=False) or whether only size, type, and modification time are compared (shallow=True, which is the default).

File hashing

I would bet that the main reason that synchronizing files is taking so long is that the program calculate the hash of every file it encounters. Specifically this line in get_task():

file_info_1[file1] = (filehash(file1_path), os.path.getsize(file1_path), os.path.getmtime(file1_path))

and the corresponding line for file2. Getting the hash of a file requires reading the entire file. Doing this for large directory trees will be slow because every file has to be read up to three times: once for file1, once for file2, and once when copying on to the other. Plus, when the files being compared are not on separate networked computers, computing a hash is just overhead plus a small chance of a hash collision.

When shallow=False, filecmp.cmpfiles() will compare the two files byte-for-byte to see if they are the same. When shallow=True, only size, type, and modification date are used to judge if two files are the same. The latter is much faster, but possibly not as accurate. But, this possible inaccuracy has a very small probability of occurring unless the user is doing weird things to their file system. If you modify a file, its modification date and (most likely) its size are going to change. That's usually enough of a clue to determine when a file needs to be synchronized.

Answer 2

Translation

sf_label_text_dic is a good idea! You have translations for your UI. But the structure of this database should change: instead of being a dictionary of lists, it should be a dictionary of dictionaries with keys of IETF language codes. Better yet, convert all of this to the gettext API which is the "official" way to support translations.

Contrast this with your code, where your variable names are in English (good) but some of your comments are in Mandarin. Your comments should also be in English, because English is the de-facto language of software development (for better or worse). In the other direction, you have UI content such as

show_toast('Sync Successfully',

that is missing translation support to Mandarin.

Paths

scan_folders_in is recursive. Definitely needs to be avoided. Replace your scandir with walk which will do the recursion for you.

You apply the resulting folders to a set(), but... is it even possible to have duplicate directory names? I think it isn't, and this can be skipped.

For operations like folder + '\\' + dI, and really any path manipulation, use the pathlib.Path API.

Misc.

get_task needs to be broken up into multiple subroutines.

Never bare except/pass. In this case that's being applied to an sf_show_notice; you need to figure out the specific error you're trying to protect against and deal with that instead.

initialization_done is an anti-pattern. By the time that __init__ is complete, initialization of the class should be done and all of the members of the class should be stored on the instance. It's still a good idea to have a separate launch() method, but this would only start your thread pool and run your main loop - nothing else.

Answer 3

surf_items = os.scandir(f_path)
...
files = [folder + '\\' + dI for dI in os.listdir(folder) if os.path.isfile(os.path.join(folder, dI))]

The advantage of os.scandir over os.listdir is that it batch-fetches extra information about each directory entry, including its type (file or directory), so you can avoid the overhead of individual calls to functions like isfile. In your code you call os.scandir and use its efficient is_dir method, but then you call os.listdir on the same directory and call os.path.isfile on every file in it.

As other people said, for what you're doing, you can probably use os.walk. But sometimes os.walk is unsuitable, and in those cases you should use os.scandir and its methods, not os.listdir and os.stat/os.path.isX.

Also, you constructed the same path in two different ways in the same expression: folder + '\\' + dI and os.path.join(folder, dI). The second is better.

surf_items = os.scandir(f_path)
...
surf_items.close()

It's better to say with os.scandir(f_path) as surf_items: .... Objects that have a close method usually support with.

folders = sorted(set(folders))

I don't understand why you're using set as there should never be any duplicates in the folder list.

Also, you're sorting the same items over and over (at each level of recursion). It would be better to make a list of subfolders, sort that, then loop over it and recurse. You might also consider not sorting at all.

[len(folder_path)::]

There should be only one : there. Two isn't wrong, but it's unidiomatic.

result = [folder_store, file_store]
return result

Normally you would return a tuple, not a list. I would just write return folder_store, file_store.

for pass_folder in pass_folder_paths.split(','):

This means that the caller can't use a folder whose name contains a comma. Python has real data structures, and you should use them here: make pass_folder_paths a list/tuple/set of strings (and the same for pass_item_rpath). If the paths have to be comma-separated somewhere (in the command-line interface, for example), put the split(',') in the code related to that, and don't infect the rest of the program with it.

path1.split('\\')[-1]

Use os.path.basename(path1).

except:

There is almost no situation in which you ever want to write except without a list of exceptions, because it catches everything. If the user presses Ctrl+C, that raises a KeyboardInterrupt, which except: will catch and suppress. If there's a typo in your code, you're likely to get a NameError, which except: will catch and suppress.

Figure out what exceptions you actually want to catch and list them.

Answer 4

The main source of slowness here will be hashing too much. Think about the situations in which it may be acceptable to skip the hashing and assume that files are identical based on metadata. There's a tradeoff between speed and possible mistakes here, so you have to decide this based how you'll use the program.

You can also speed up the hashing a bit, but the gains from this will be more limited. Nevertheless, try switching the hash function to SHA1 and using a larger block size, something between 16 KiB and 128 KiB. It should be faster on modern CPUs.

However, the main problem with your program is insufficient abstraction.

• You have repetitive code all over the place. For example,

  toaster.show_toast(
      ...,
      icon_path=mf_data_path + r'Movefile.ico',
      duration=10,
      threaded=False)
  

  Create more variables and functions to avoid repeating yourself and make the code more readable.

• You're also slicing and dicing paths with string operations everywhere. Even if Python didn't have path functions, it would be a good idea to create a module containing your own path utilities with descriptive names. Fortunately, you don't have to do this because Python comes with two modules for operating on paths: os.path if you prefer to work with strings, and pathlib if you'd rather handle paths as objects. Pick one of these and use it consistently for all paths in your program. If you need something that these modules don't have, try to build it out of their functions instead of string operations.

• Your directory scanning function is a reinvented version of os.walk, so use that instead.

• You're using tuples and lists a lot to bundle a few items together. Try creating some dataclasses instead. It's more self-documenting.

• For translating your program, you have gettext in Python's standard library.

• You may want to separate the UI code from the file manipulation logic. Callbacks or generators will be helpful here.

And one more thing, your code suffers from a steady march to the right. I recommend you split long lines more and get rid of all alignment, which will help you stay within a 80 character limit. For example, instead of

def long_function_name(long_argument, another_long_argument, a_third_long_argument='default_value'):
    some_nice_class.a_long_method_name(a_variable,
                                       'This is an example string that will be passed to the method')

try

def long_function_name(
        long_argument,
        another_long_argument,
        a_third_long_argument='default_value'
):
    some_nice_class.a_long_method_name(
        a_variable,
        'This is an example string that will be passed to the method')

Answer 5

Please do yourself a favor and change your naming conventions:

...
for file1 in all_files_1:
    file1_path = path1 + file1
    file_info_1[file1] = (filehash(file1_path), os.path.getsize(file1_path), os.path.getmtime(file1_path))

for file2 in all_files_2:
    file2_path = path2 + file2

    if file2 in all_files_1:
        file1 = file2
        file1_path = path1 + file1
        file_info = file_info_1[file1]
        file2_info = (filehash(file2_path), os.path.getsize(file2_path), os.path.getmtime(file2_path))

        if file_info == file2_info:
            continue

        if single_sync and file_info[0] == file2_info[0]:
            continue

        new_file, old_file = file1, file2
        ...

Variable names such as file1, all_files_1 are not descriptive at all, and make comprehension more difficult. If you gave the variables more meaningful names, the code would become slightly more clear. In fact, at the end of this snippet you use: new_file, old_file. I would use: source_file, target_file. But at least it becomes more clear.

Variables

Unused variable: task_number. You are incrementing it but not doing anything with it seems. Remove all the bloat.

One of your problems is that you have too many variables, which makes it more difficult to understand and maintain the code, and it also increases the risk of bugs. Using the Pathlib library you could easily get file parts or properties such as size without resorting to multiple variables. And it will make your code cross-platform, at least for the handling of files.

Translations

As said previously, you may want to consider I18N-based modules to hold your translations. See: Multilingual internationalization services Then you can either use JSON files, or .po files, that can be maintained with tools such as poedit.

There is nothing wrong here, but if the number of strings was more significant, the use of dict would show its limits. And you already have two dicts for no good reason.

Fine-tuning

Since your ThreadPoolExecutor is declared like this:

with ThreadPoolExecutor() as executor:

which has this signature:

def __init__(self, max_workers=None, thread_name_prefix='',
             initializer=None, initargs=()):

The number of workers will be determined according to this formula:

if max_workers is None:
    # ThreadPoolExecutor is often used to:
    # * CPU bound task which releases GIL
    # * I/O bound task (which releases GIL, of course)
    #
    # We use cpu_count + 4 for both types of tasks.
    # But we limit it to 32 to avoid consuming surprisingly large resource
    # on many core machine.
    max_workers = min(32, (os.cpu_count() or 1) + 4)

The result is 16 in my case. You should try with different values, since there is no guarantee that the default value will be most optimal.

Benchmarking

Do the same job with a tool like rsync, which we can assume is highly optimized. Measure execution time, then you can figure out what a realistic run time would be. Copying a lot of files is inevitably going to take some time, not to mention the hashing. But right now, it's hard to tell how bad your program is in terms of performance. If it takes 1 hour to run whereas rsync does the exact same thing in 10 minute, then you have a clear goal to attain.

Suggested reading

Multithreaded File Copying in Python For some ideas like changing the number of thread pools or trying different libs.