Python script to rename images based on descriptions in a text file

Question

I like to collect images for my desktop background, but the problem is sometimes the image names don't represent what the image is. I decided to write a script that reads a text file that contains the source of the images (folders stored in the Pictures directory under the Userprofile (windows)) to be renamed.

The full path to the folder listed below are:

C:\Users\Kiska\Pictures\Landscape
C:\Users\Kiska\Pictures\Batman

Rather than have the user type the useprofile path every time they want to add a folder to the list, the Folder.py class does it for them, they just provide the sub directory to be processed.

Source (.txt):

Landscape
Batman

PathUtilities.py:

def verify_parent_directory(parent: str) -> None:
    parent = parent.strip()
    path_without_drive_letter = parent[2:]

    _determine_if_drive_letter_is_valid(file_path=parent)
    _check_if_string_ends_with_slash(string_to_validate=path_without_drive_letter)

    if len(path_without_drive_letter) > 2:
        _check_for_invalid_characters(string_to_validate=path_without_drive_letter)


def verify_subdirectory(subdirectory: str) -> None:
    subdirectory = subdirectory.strip()
    _check_if_string_starts_with_slash(string_to_validate=subdirectory)
    _check_for_invalid_characters(string_to_validate=subdirectory)
    _check_if_string_ends_with_slash(string_to_validate=subdirectory)


def _determine_if_drive_letter_is_valid(file_path: str):
    drive_letter_with_colon = file_path[:2]

    if not drive_letter_with_colon[0].isalpha():
        raise TypeError("Drive Letter is invalid.")

    if drive_letter_with_colon[1] != ":":
        raise ValueError(f"Second element is invalid. Character(s): {drive_letter_with_colon[1]}")


def _check_for_invalid_characters(string_to_validate : str):

    """
    Determine if the string contains forbidden elements.

    Raises a ValueError if any forbidden character is found.

    Args:
        string_to_validate : str - The string we're going to test.
    """

    forbidden_characters = ["<", ">", ":", "/", '"', "|", "?", "*", "\\\\"]

    for forbidden_character in forbidden_characters:
        if forbidden_character in string_to_validate:
            raise ValueError(f"Invalid characters in path. Character(s): {forbidden_character}")


def _check_if_string_starts_with_slash(string_to_validate : str):
    if string_to_validate.startswith("\\"):
        raise ValueError("Invalid characters in path. Character(s): \\")


def _check_if_string_ends_with_slash(string_to_validate : str):
    if string_to_validate.endswith("\\"):
        raise ValueError("Invalid characters in path. Character(s): \\")

I created the above module because I might have more projects that require validation of paths.

Folder.py:

import pathutilities
import os


class Folder:

    def __init__(self, parent: str, subdirectory: str):
        pathutilities.verify_parent_directory(parent=parent)
        pathutilities.verify_subdirectory(subdirectory=subdirectory)

        self._parent = parent
        self._subdirectory = subdirectory

    @property
    def parent(self):
        return self._parent

    @property
    def subdirectory(self):
        return self._subdirectory

    def construct_path(self) -> str :
        return os.path.join(self._parent, self._subdirectory)

    def __eq__(self, other):
        if isinstance(other, Folder):
            return (self.parent, self.subdirectory) == (other.parent, other.subdirectory)
        return NotImplemented

    def __hash__(self):
        return hash((self._parent, self._subdirectory))

Repository.py:

from Folder import Folder
import os
import shutil


class Repository:

    def __init__(self, source: Folder, destination: Folder):
        if source == destination:
            raise ValueError("Source folder cannot be the destination folder")
        self._source = source
        self._destination = destination

    def copy_files_with(self, extension: str):
        if extension.startswith("."):
            raise ValueError("extension doesn't require a period")

        source = self._source.construct_path()
        destination = self._destination.construct_path()
        number_of_images_in_source = self._get_number_of_images_in_directory(directory=source)

        if number_of_images_in_source:
            print(f"Copying images from {source} to {destination}\nNumber of images: {number_of_images_in_source}")
            os.makedirs(destination, exist_ok=True)

            number_of_images_in_destination = self._get_number_of_images_in_directory(directory=destination) + 1

            for number, image in enumerate(os.listdir(source), start=number_of_images_in_destination):
                if image.endswith(extension) or image.endswith(extension.upper()):
                    source_image = os.path.join(source, image)
                    destination_image = os.path.join(destination,
                                                     self._construct_destination_string(current_number=number,
                                                                                        extension=extension))
                    print(f"Copying {source_image} to {destination_image}")
                    shutil.move(source_image, destination_image)
        else:
            print("No images to copy")

    def _get_number_of_images_in_directory(self, directory: str) -> int:
        return len(os.listdir(directory))

    def _construct_destination_string(self, current_number, extension):
        return "{0}_{1}.{2}".format(self._source.subdirectory.lower().replace(" ","_"), current_number, extension)

Main.py:

import sys
import os
from Folder import Folder
from Repository import Repository


def main():

    try:

        source = "{0}\\{1}".format(os.getenv("USERPROFILE"), "Pictures")
        destination = "R:\\Pictures"

        source_list = "source.txt"

        with open(source_list) as folders_from_source:
            for subfolder in folders_from_source:
                subfolder = subfolder.strip()
                source_folder = Folder(parent=source, subdirectory=subfolder)
                destination_folder = Folder(parent=destination, subdirectory=subfolder)
                repository = Repository(source=source_folder, destination=destination_folder)
                repository.copy_files_with(extension="jpg")

    except (TypeError, ValueError, FileExistsError) as error:
        print(error)
    finally:
        sys.exit()


if __name__ == '__main__':
    main()

Suppose there were two images in each of the source folders, it will rename them like this:

landscape_1.jpg
landscape_2.jpg

batman_1.jpg
batman_2.jpg

Areas of Concern:

• Is my code clean? Descriptive variable and method methods, small classes and at least for me, it's easy to follow.

• I didn't include docstrings to save space, but I'm well aware I should include them.

Answer 1

First, I'm afraid all the path handling and validation is a common anti-pattern of trying to check every conceivable error condition before trying to do something, in order to avoid lots of error-handling code. The problem is it doesn't work - it is literally impossible to guard against all possible errors, because an error may be introduced after you verify that things are fine, and before you act on that information. For example, you may check for the existence of a directory, but something or something removes or replaces it before you have a chance to use it. So my primary suggestion would be to simply remove all of PathUtilities.py and Folder.py, and to use the file access tools directly in your main code. What will happen then is if you attempt to do something like read a non-existing file you will get an informative uncaught exception from the Python standard library, and those will be easy to debug and/or handle when you see them.

That said:

1. Repository.py and Main.py belong in the same file, at least until the program becomes a fair bit more complex. This is a common pattern in small Python utilities, since there is a big advantage to having a program be a single file as opposed to several.
2. Don't worry about docstrings. If you make your code sufficiently easy to read they just clutter up the place in my experience.
3. sys.exit() is redundant as it stands. To make it useful you can pass it a number to indicate success or failure of the run. By convention zero indicates success, one often indicates an unknown error, and other numbers indicate application-specific errors. Don't use numbers above 255; exit codes are just a single byte on common platforms. A common pattern here is to sys.exit(main()) at the bottom of the file, and have main return an int.
4. black can automatically format your code to be more idiomatic.

5. flake8 with a strict complexity limit will give you more hints to write idiomatic Python:

   [flake8]
   max-complexity = 4
   ignore = W503,E203
   

   That limit is not absolute by any means, but it's worth thinking hard whether you can keep it low whenever validation fails. For example, I'm working with a team on an application since a year now, and our complexity limit is up to 7 in only one place.

6. I would then recommend adding type hints everywhere and validating them using a strict mypy configuration:

   [mypy]
   check_untyped_defs = true
   disallow_untyped_defs = true
   ignore_missing_imports = true
   no_implicit_optional = true
   warn_redundant_casts = true
   warn_return_any = true
   warn_unused_ignores = true
   

In general the code is easy to read, but could use some simplifying.