I'm trying to organize my gallery, and I have certain folders rife with copies.

My daily work is mostly javascript and php, being your generic web developer. However I took this as an opportunity to bodge something in python, a language I normally do not use, but have always wanted to pick up at least a little bit. You easily bodge things in it - just open a file and write shit, and evidently has absolutely marvellous standard library for my purposes here (heavy, weird filesystem operations).

Anyway, just for reference, a lot of what I'm trying to do overlaps with this (how I found this site in the first place): Replacing duplicate files with hard links but I do a lot of my own things.

There's 2 goals here:

1. Build a set of tools/functions to easily let me organize shit.
2. In this specific case, dedupe the files in a specific directory by making them hardlinks to a different source directory.

Code is below:

#!/usr/bin/python3

import sys
import zlib
import os
import hashlib
import imghdr

'''
1. Make file things
2. Build cross-reference tables.
3. Take all the matching hashes from the source and target.
if hashes match, check if hardlink between source and target

TODO?: Can use WSL/bash to do a reliable file type check - using the file command.
'''

source_dir = r'C:\!The Gallery'
target_dir = r'C:\Games\PnP DnD'
only_these_extensions = ['jpg', 'png', 'gif', 'jpeg', 'bmp', 'tif', 'tiff', 'webp']

# Define a main() function that prints a little greeting.
def main():

print('Finding files: ', end='')
source_files = get_dir_contents(source_dir, extensions=only_these_extensions)
target_files = get_dir_contents(target_dir, extensions=only_these_extensions)
print('Done')
print('Collected ' + str(len(target_files) + len(source_files)) + ' files.')

file_things = make_file_things(target_files + source_files)

(hash_table, node_table, name_table) = build_tables(file_things)
print(len(hash_table))
print(len(node_table))
print(len(name_table))

# Hardlink from target to source
for thing in file_things:
# Take all the target files
if os.path.commonpath([target_dir, os.path.split(thing.fpath)[0]]) == target_dir:
target_thing = thing
if len(hash_table[thing.fhash]) > 1:
# Get all hash matches that matched more than one file.
for hash_matched_thing in hash_table[thing.fhash]:
# Get any matches in the source dir.
if os.path.commonpath([source_dir, os.path.split(hash_matched_thing.fpath)[0]]) == source_dir:
source_thing = hash_matched_thing

return

def make_file_things(file_paths):
file_things = []
for file_path in file_paths:
with open(file_path, "rb") as file:
digest = crc32(content)
file_things.append(file_thing(file_path, os.stat(file_path), digest))
return file_things

def build_tables(files):
hash_table = {}
node_table = {}
name_table = {}

print('Building reference tables:', end='')
# i = 0
for file in files:
# progress('Building tables', i, len(files), 25)
# i = i + 1
hashy = file.fhash
inode = file.fstat.st_ino
name = os.path.split(file.fpath)[1]

if hashy in hash_table:
hash_table[hashy] = file
else:
hash_table[hashy] = [file]

if inode in node_table:
node_table[inode] = file
else:
node_table[inode] = [file]

if name in name_table:
name_table[name] = file
else:
name_table[name] = [file]

print(' Done!')
return (hash_table, node_table, name_table)

#################################################
# Helpers
#################################################
class file_thing:
fpath = ''
fstat = 0
fhash = 0

def __init__(self, fpath, fstat, fhash):
self.fstat = fstat
self.fpath = fpath
self.fhash = fhash

def __str__(self):
return str([self.fpath, self.fhash, self.fstat])

def __repr__(self):
return str([self.fpath, self.fhash, self.fstat])

def enum_hardlinks(dupe_files):  # Expects all files given to be duplicates
for dupe in dupe_files:
file_index = os.stat(dupe).st_ino  # Inode on unix, file index on windows.
else:

# The old version, before I knew about st_ino
def enum_hardlinks2(dupe_files):  # Expects all files given to be duplicates
for dupe in dupe_files:
done = False
if (os.path.samefile(hl_set[0], dupe)):
hl_set.append(dupe)
done = True
break
if (not done):

try:
extensions = [ext.lower() for ext in extensions]
except TypeError:
pass
result = []
allfiles = os.listdir(dir)
# num = len(allfiles)
for obj in allfiles:
if (root is None):
fullpath = dir + '\\' + obj
ext = get_ext(fullpath)
if (extensions is None or ext in extensions):
result.append(fullpath)
else:  # TODO: Following symlinks outside the root here can get nasty. Need to handle.
relpath = dir[len(root)+1:] + '\\' + obj
fullpath = root + '\\' + relpath
ext = get_ext(fullpath)
if (extensions is None or ext in extensions):
result.append(relpath[1:])
return result

def enum_extensions(files, exclude_extensions=None):
ext_set = set()
for file in files:
ext = get_ext(file)
if (exclude_extensions is None or ext not in exclude_extensions):
return ext_set

def group_by_extension(files, exclude_extensions=None):
ext_dict = {}
for file in files:
ext = get_ext(file)
if (exclude_extensions is None or ext not in exclude_extensions):
if (ext in ext_dict):
ext_dict[ext].append(file)
else:
ext_dict[ext] = [file]
return ext_dict

# Fails in ~8% of cases. Meh.
def unreliable_filetype_check(files):
type_dict = {}
mismatches = []
for file in files:
ext = get_ext(file)
img_type = imghdr.what(file)
if (ext == 'jpg'):
ext = 'jpeg'
if (ext == 'tif'):
ext = 'tiff'

if (img_type is not None and ext != img_type):
mismatches.append(file)
print(ext + ' vs ' + img_type)
if (img_type in type_dict):
type_dict[img_type].append(file)
else:
type_dict[img_type] = [file]
print(str(len(type_dict[None])) + ' images whose type could not be determined.')
print(type_dict[None])
return mismatches

def crc32(data):
return hex(zlib.crc32(data) & 0xffffffff)

def sha265(data):
return hashlib.sha256(data).hexdigest()

def get_ext(file):
return os.path.splitext(file)[1][1:].lower()

'''
The point is to only print when we are very close to a value we want. But we can't guarantee equality,
so we have to use some kind of range around which we trigger the print. The upper bound for this range
is 1/total, half below and half above our real value. There's edge cases where we end up with 2
consecutive values within our range, but those are rare. Basically on odd numbered items, around 50%,
when we have enough precision to accurately represent both ends of the range.
'''
def progress(prefix, current, total, step=1):
temp = current / total * (100/step)
delta = 1 / total * (50/step)  # Half above and below each target value, for a total range of 1/total
if abs(temp - round(temp)) > delta and current != total-1:
return
temp = temp*step  # Mult by step again to get back to a scale of 100
# Random edge case where both the nearest below and above values end up in our target range
if (round(temp) == 50 and temp - round(temp) < 0):
return
if env == 'sub':
if current == 0:
print(prefix + ': ' + '{:2.0f}'.format(temp) + '%', end='')
elif current == (total-1):
print(' ' + '{:2.0f}'.format(temp) + '%')
else:
print(' ' + '{:2.0f}'.format(temp) + '%', end='')
sys.stdout.flush()
elif env == 'cli':
if round(temp) > progress:
print('Processing: ' + '{:2.0f}'.format(temp) + '%', end='\r')
progress = round(temp)

# This is the standard boilerplate that calls the main() function.
env = ''
if __name__ == '__main__':
if (sys.version_info[0] < 3):
raise Exception('Your frickin python version is so very WROOOOOOOONG')
try:
os.environ['PYTHONSUBLIMEBUILDSYSTEM'] #If this exists, we're in our custom build sublime environment. Plz don't try to ask for input here.
env = 'sub'
except:
env = 'cli'
main()



What I'm looking for is:

• advice on the general approach - e.g. the algorithm on line 41+ could probably use work (below the Hardlink from target to source comment). I wonder if I'm missing any considerations here.
• performance - I think I can get away with crc32, which is fast. Def don't need cryptographically strong hashing tho. Also, any bit of performance is welcome - the size of the input is 120K items and 150GB in size, and it's only gonna grow.
• organizing code - pretty new to python. This is maybe the third thing I've made that's more complex than 5 lines of code in python. Probably not gonna use it in any professional capacity any time soon, but any advice for "bodge projects" welcome.
• libraries - finally, an alternative to imghdr would be welcome. Chokes on way too many images.

Personal idiosyncrasies: I'm used to line lengths of 120 characters (inb4 someone says stick to 80).

• Welcome to Code Review. I have rolled back your latest edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. – Heslacher Jun 25 at 5:53
• Ah, ok. I suppose that means I should to post a new question with the revised code...? – martixy Jun 25 at 6:00
• You can give this question a little bit more time as well. Here on Code Review answers usually take mor time. – Heslacher Jun 25 at 6:06
• Cool. What is a reasonable time to wait before it's a good idea to post an updated version? – martixy Jun 25 at 6:09
• Give it one more week. – Heslacher Jun 25 at 6:10

I was interested in this as it's a cool idea. However the demeanor in your question and code makes me not want to review this.

Your frickin python version is so very WROOOOOOOONG

That's really mature.

get_dir_contents is just a verbose version of os.walk. What's worse is root is unused and bizarre. If you change only_these_extensions to start with a dot then you can also further simplify your code.

import os
import pathlib
from typing import Set, Iterator

only_these_extensions = {'.jpg', '.png', '.gif', '.jpeg', '.bmp', '.tif', '.tiff', '.webp'}

def get_dir_contents(path: str,
extensions: Set[str],
) -> Iterator[pathlib.Path]:
dir_path = pathlib.Path(dir_path)
for file_name in file_names:
full_path = dir_path / file_name
if full_path.suffix in extensions:
yield full_path


After this I started looking at the rest of your code and I didn't get far until I saw build_tables which looks to have a massive bug. node_table[inode] = file should probably be node_table[inode].append(file). And unhelpful classes called file_thing.

Change your code to use pathlib, os.path and mypy, remove all your useless prints and remove your "I'm better than you" demeanor and I'd give you a better followup review.

• First, thanks for taking the time. But you have me thoroughly confused. <"I'm better than you" demeanor>? You will have to elaborate because I have no idea what you are referring to. Can't remove it if I don't know what to remove. If you're referring to that version comment - that is a comment to myself and no one else. I've removed it now. pathlib is quite the neat library, and thanks for that. I think for the couple of lines of elementary path manipulation, os.path is enough for now. Cont'd... – martixy Jun 25 at 5:44
• mypy is an interesting idea, but I don't think I have a use for it in this instance. It's a hassle to setup and it isn't worth it for these types of projects, where you're just prototying things. It's also additional syntax baggage who someone new at python doesn't need right now. Someone actually removed the beginner tag from the question, and I'm not sure why TBH. I really am a beginner on python. Onto code comments... – martixy Jun 25 at 5:45
• Thanks for pointing out os.walk. I remember I made get_dir_contents for a different script, just copied here, hence the unused root. build_tables -> valid bug, should be .append(). file_thing is named as such, because there are already enough identifiers named file, and I didn't want to overload the word any further. Renamed to file_meta, hopefully that makes it clearer. Finally, I have no idea what you mean by useless prints. I want some kind of feedback while the program is running. What else do you suggest? – martixy Jun 25 at 5:45
• @martixy Go over your comments and strings and remove any others that are just rude - like the example I gave. If you'd be embarrassed to say it to your mom, it's probably not a good idea to put it in your program. mypy would have found the .append() bug, this says to me that it's not useless. Yes it has a bit of a learning curve, but given that C, C# and Java all use static typing and beginners learn those I don't think mypy it not beginner friendly, you can even tell it to not type things if it's out of your depth. – Peilonrayz Jun 25 at 10:51
• Point to any other example. You failed to elaborate and keep getting hung up on that single line. And how would mypy help with that bug? I did try it. It detected nothing. As much as I appreciate bringing mypy to my attention, I choose to use vanilla python here. Someone pointed out that I shouldn't be editing the code, so the question remains as is. I did however rewrite get_dir_contents as you suggested, and fixed a couple of bugs, append included. Still looking for performance tips. The largest bottleneck is remains in the file read and hashing phase. – martixy Jun 25 at 19:26