Simple Python Code that zips file

Question

My Code

import shutil
import os
from pathlib import Path
from datetime import datetime


DOWNLOAD_DIR = Path("/Users/my_name/Desktop/Zip Test/g_2020-12-10_3/")
ZIPPED_DIR = Path("/Users/my_name/Desktop/Zip Test/g_2020-12-10_3.zip")
ZIPPED_DIR.mkdir(exist_ok=True)


def make_archive(source, destination):
    delivery_file_name = destination.split("/")[-1]
    delivery_name = delivery_file_name.split(".")[0]
    format = delivery_file_name.split(".")[1]
    archive_from = os.path.dirname(source)
    archive_to = os.path.basename(source.strip(os.sep))
    shutil.make_archive(delivery_name, format, archive_from, archive_to)
    shutil.move(
        "%s.%s" % (delivery_name, format),
        "/Users/my_name/Desktop/Zip Test/zipped_content",
    )


make_archive(str(DOWNLOAD_DIR), str(ZIPPED_DIR))

Description

I am very new to python so i had a very hard time getting shutil.make_archive to do what i want. This code is finally working and i feel it is a little hard to read so i was wondering if someone can help me refactor this to simplify it. I don't want to loose the functionality of what i am doing. In my code i basically go to my /Users/my_name/Desktop/Zip Test and inside it is a folder called g_2020-12-10_3 which contains the contents i want to zip up so i do that.

When i unzip the contents it unzips folder named g_2020-12-10_3 and inside it are the contents.

I want to keep this functionality but at the same time simplify the code below and any help would be appreciated here.

Answer 1

You're using pathlib, which is great! Unfortunately, you immediately discard the functionality it gives you, and use old-style os APIs.

A good place to start is here - a mapping between the os[.path] APIs and their Path equivalents. Doing that gets us here:

def make_archive2(source, destination):
    shutil.make_archive(destination.stem, destination.suffix, source.parent, source.name)
    shutil.move(destination, "/Users/my_name/Desktop/Zip Test/zipped_content")

We now don't need to do any manual manipulations using os or os.path. Once we're here, we can actually realize that the first parameter to make_archive is a full path to the file, completely removing the requirement to call shutil.move

From there, we can improve some of the other things you do. For example:

• Your source and destination are the same thing... but you don't actually use the destination in a meaningful way
• You extract the format from the destination, which feels a bit odd
• You hardcode the actual final destination
• You use both the root_dir and base_dir parameters, when that really isn't necessary
• You don't return where the file ended up, which feels nicer than the caller having to set up their Path themselves

Altogether, I ended up with something like this:

def make_archive3(to_archive, move_to, archive_format="zip"):
    move_to.mkdir(exist_ok=True)
    return shutil.make_archive(move_to / to_archive.name, archive_format, to_archive)