Rename and move images to a destination directory based on a text file followup

Question

This is my third (and hopefully final) time re-writing this program. It was recommended I use Pathlib and it simplified my code, it's easier to read and understand than the first two monstrosities.

I also changed the way it functions, instead of the user providing a text file, the user creates a directory called Repository, which contains sub directories of images that will be renamed, and a destination directory (R:\Pictures for me).

Suppose I have the following directories under Repository:

C:\Users\Kiska\Repository\Computers
 - comp-amd-12342.jpg
 - 7546345233-pcmag.jpg
C:\Users\Kiska\Repository\Landscape
 - land-345345.jpg
 - end-field.jpg

The program will move and rename it like this:

R:\Pictures\Pictures\Computers
 - computers_1.jpg
 - computers_2.jpg
R:\Pictures\Pictures\Landscape
 - landscape_1.jpg
 - landscape_2.jpg

Code:

import os
from pathlib import Path
import shutil
def check_if_folder_exists(folder : Path):
 if not Path.exists(folder):
 raise FileNotFoundError("{0} is not available".format(folder))
def main(): 
 try:
 repository = Path("{0}\\{1}".format(os.getenv("USERPROFILE"), "Repository"))
 desination_root = Path("R:\\Pictures")
 check_if_folder_exists(folder=repository)
 check_if_folder_exists(folder=desination_root)
 for folder in repository.iterdir():
 destination = Path(desination_root).joinpath(folder.stem)
 Path.mkdir(destination, exist_ok=True)
 start = len(os.listdir(destination)) + 1
 for number, image in enumerate(folder.iterdir(), start=start):
 image_name = "{0}_{1}{2}".format(folder.stem.lower().replace(" ","_"), number,image.suffix)
 destination_image = Path(destination).joinpath(image_name)
 print(f"{image} will be named {destination_image}")
 shutil.move(image, destination_image)
 except(ValueError, FileNotFoundError, OSError) as error:
 print(error)
 finally:
 pass
if __name__ == '__main__':
 main()

Answer 1

• You have lots of unneeded empty lines, they also hinder readability.
• fn(arg=value) is abnormal. It's common to use fn(value). It's common only to use the former when the argument is a keyword or default argument.

• You don't need to make a path another path.

  desination_root = Path("R:\\Pictures")
  destination = Path(desination_root).joinpath(folder.stem)
  

• Path.mkdir isn't a static method, and so it's abnormal to use it as Path.mkdir(my_path) rather than my_path.mkdir().

• check_if_folder_exists isn't needed as repository.iterdir and destination.mkdir will error if they don't exist.

  It should also be noted that you can pass parents=True to Path.mkdir to automatically create the folder if it doesn't exist, rather than force the user to do that.

• It's better if you make another function that moves the files, and one, main, that passes it the values. Preferably main would be changed to not use static values and get the values from a config file.
• finally isn't needed.
• You should raise a SystemExit(1) or use sys.exit(1) if there is an error. They both do the same thing which notifies the system that the program failed. It didn't run without problems.

• Given that generating a name that doesn't conflict with existing files is not a simple one-liner I'd recommend you move it out into a function.

  I should note that it may overwrite files, if you have a folder with file_1.png and file_3.png your code's going to overwrite file_3.png, but not have a file_2.png.

You changed away from some of the above, and so I'd recommend you think about the differences, which is easier to re-use, which is easier to read, which performs the correct action in the situation. If you don't know what the difference is even with my explanation, such as with SystemExit(1), then you should look up what it does.

import os
from pathlib import Path
import shutil
def gen_name(path, number):
 orig_name = path.parent.name.lower().replace(" ", "_")
 return f"{orig_name}_{number}{path.suffix}"
def move_files(src, dest):
 for folder in Path(src).iterdir():
 destination = Path(dest) / folder.stem
 destination.mkdir(exist_ok=True, parents=True)
 start = len(os.listdir(destination)) + 1
 for number, image in enumerate(folder.iterdir(), start=start):
 image_name = gen_name(image, number)
 destination_image = destination / image_name
 print(f"{image} will be named {destination_image}")
 shutil.move(image, destination_image)
def main():
 try:
 move_files(
 "{0}\\{1}".format(os.getenv("USERPROFILE"), "Repository"),
 "R:\\Pictures",
 )
 except(ValueError, FileNotFoundError, OSError) as error:
 print(error)
 raise SystemExit(1) from None
if __name__ == '__main__':
 main()

I think it could make more sense for gen_name to be a closure, as then it can correctly handle existing files. I won't change how the code functions, but will show how it could be implemented.

def gen_name_builder(folder):
 orig_name = folder.name.lower().replace(" ", "_")
 number = len(os.listdir(destination))
 def gen_name(file):
 nonlocal number
 number += 1
 return f"{orig_name}_{number}{path.suffix}"
 return name
def move_files(src, dest):
 for folder in Path(src).iterdir():
 destination = Path(dest) / folder.stem
 destination.mkdir(exist_ok=True, parents=True)
 gen_name = gen_name_builder(destination)
 for image in folder.iterdir():
 image_name = gen_name(image)
 destination_image = destination / image_name
 print(f"{image} will be named {destination_image}")
 shutil.move(image, destination_image)

• киска: Is it okay to use an if/else statement in the for loop responsible for moving the images? Check if it exists, if not, move it, if it does, leave it, but notify the user with a print statement
• Peilonrayz: @Kiska Please look over the additional code I added to my answer. You should be able to handle if an item exists with that name in gen_name. Yes, you can use an if. As a hint I would use a while.
• Peilonrayz: @Kiska I'm not seeing the docs use Path.mkdir as a static method. I've checked the other languages too, and they also don't show that. I'm sorry that this has caused some confusion.
• киска: Right at the top in yellow where you can link to it Path.mkdir(mode=0o777, parents=False, exist_ok=False).
• Peilonrayz: @Kiska Ok, I get the confusion now. That isn't an example of how to use the method, it tells you where the method is defined, Path.mkdir, and shows the arguments it has, with any defaults they have. If you look at Path.open you can see the same thing, but it has an example showing it used in a different way.

Answer 2

• Give main method a more meaningful name. It is always better than a generic main.
• Make repository and destination_root be parameters of the method which is now called main;
• If you use python version that supports f-strings, be consistent, use them everywhere e.g. f'{folder} is not available';
• finally: pass does not have any effect so if you are not going to put any logic there, it can be omitted;
• regarding the start parameter passed to enumerate: can you guarantee that if that the destination folder contains 1 picture, it will be called xxxxx_1.jpg, not xxxxx_2.jpg or xxxxx_27.jpg?
• I would also extract folder.stem.lower().replace(" ","_") into a separate variable which can get some meaningful name (which can be then used in f-string) to enhance reading;

• киска: The start parameter is the number of images already present + 1 so if 4 images already exist in the destination, we start at 5.
• Hlib Babii: @Kiska Okay, let me rephrase, what would happen if before you run the program R:\Pictures\Pictures\Computers contained one file with the name computers_2.jpg?
• киска: Ah! I see now what you mean. I should check first if the filename is already present in the destination directory.