First things first:
You can break tasks like this and increase readability dramatically.
OLD:
if first_check:
new_name = file_name[first_check.span()[1]:]
shutil.move(os.path.join(dirpath, file_name),
os.path.join(dirpath, new_name))
else:
new_name = file_name[second_check.span()[1]:]
shutil.move(os.path.join(dirpath, file_name),
os.path.join(dirpath, new_name))NEW:
if first_check:
new_name = file_name[first_check.span()[1]:]
else:
new_name = file_name[second_check.span()[1]:]
shutil.move(os.path.join(dirpath, file_name),
os.path.join(dirpath, new_name))You are solely changing the file_name to a new_name in the if/else statement, you do not need to have the long function call inside the if/then statement.
Next, define your globals:
FIRST_CHECK = re.compile("^\d\d - ")
SECOND_CHECK = re.compile("^\d\d ")
Next, regex matches automatically trigger an if statement, so you can remove:
if first_check != None:And replace is with:
if first_check:Next is restructuring your code: your second function is long and unwieldy. Break it up. Change move_music to 2 functions:
def move_music(source_dir, destination, dirpath, a_file):
check = check_names(dirpath, a_file)
dir_name = os.path.split(dirpath)[-1]
if dir_name != source_dir:
check_folders(destination, dir_name)
if os.path.join(source, dir_name) not in COPIED_DIRECTORIES:
manpreet![Tuteehub forum best answer]()
Best Answer
2 years ago
I created a script to walk over my directory and move my music files to my music folder while renaming the files using the regular expression library to check if the files are numbered (as I find this to be an annoyance). The script seems to be working fine, as I haven't encountered any errors, but was wondering if there was a cleaner way to do this as this seems a bit convoluted and unpythonic. (Striving to write cleaner code). By cleaner I'm not asking anyone to rewrite my entire code block(s), but rather the check_names function and it's implementation when moving files. If there are other errors those would be helpful to be pointed out for the below reason.
If this is terrible code I apologize for any python "sins" I may have committed as I have only used the os and regular expression modules a few times before.
Since i am learning how to use these and have little exposure at this point, an explanation of why would go a long way to aid in my understanding. All the
os.path.join(....)was my attempt at making it cross-platform and not having to hardcode paths. The reason I say this seems a bit convuluted is due to the runtime it takes approximately 1-2 min to do the following on say 6-7 folders: Extract the compressed.ziparchives into my original directory walk over these, rename the files if needed, and then finally moved them and then go back to the original directory and delete the remnants of the move, or is this a normal runtime for everything that's going on? (The archives are ~100-300 mb)The relevant functions are.