2
\$\begingroup\$

The program should backup files in a source folder to a zip file in destination, and keep track of how many versions there are (which explains the shelve). In order to copy these files, apparently I have to change the cwd to the source path (folder), because otherwise the program will not detect them. While it may not seem to cause that much of an issue, my OCD triggers when I see the shelve files in the new .zip folder. I thought about adding lines to delete the shelve files, but I was wondering if there is some other way.

(This program is a project in a certain book I'm reading, and while the project demanded a more basic version of this, I like to take it a step forward.)

import os, shelve, zipfile

def backupToZip(folder, destination, copies=1):

#Folder
    fName= os.path.basename(folder)

#Shelfing for copy number
    namesNums = shelve.open('names')
    if fName not in namesNums:
        num= 1
        namesNums[fName]= num
    else:
        num= namesNums[fName]

#Producing zips
    os.chdir(folder)
    for i in range(int(copies)):
        zipPath= os.path.join(destination, fName+'_'+str(num)+'.zip')
        theZip= zipfile.ZipFile(zipPath, 'w')

        for foldername, subfolders, filenames in os.walk(folder):
            for filename in filenames:
                theZip.write(filename)
        num+=1

    namesNums[fName]= num
    namesNums.close()
\$\endgroup\$
2
  • 1
    \$\begingroup\$ (Welcome to CR!) The title of your question does not comply with State what your code does in your title, not your main concerns about it. While any aspect of the code posted is fair game, it may be useful to pose explicit questions in the question body. \$\endgroup\$ – greybeard Jan 1 '18 at 8:06
  • \$\begingroup\$ @greybeard Thank you for the welcome! I will keep that in mind in my future posts/questions (will edit this one as well later on). \$\endgroup\$ – Skullz Jan 2 '18 at 6:45
2
\$\begingroup\$
  • shelve supports all methods that dictionarys do.

    Shelf objects support all methods supported by dictionaries. This eases the transition from dictionary based scripts to those requiring persistent storage.

    And so you can use dict.setdefault, rather than your if-else.

  • Rather than using num += 1, you can use range(num, num + copies). Since num is defined in an outer scope, you can use it after the for loop too.

  • Rather than using string concatination, you can use f-strings in Python 3.6+ or format strings:

    # yours
    fName+'_'+str(num)+'.zip'
    # f-string
    f'{fName}_{num}.zip'
    # string format
    '{}_{}.zip'.format(fName, num)
    '{fName}_{num}.zip'.format(fName=fName, num=num)
    
  • It's simpler to use with when handling content managers. And so you should use them on shelve.open and zipfile.ZipFile.

    Not only does it make you not have to learn how to close all types of context managers. The context will be closed in the case of an error too.

  • Don't use CamelCase or camelCase, instead use snake_case.

  • Use _ as a 'throw away' variable. Some people, however, like to instead use __.
  • Put a space either side of your equal signs. a = b, not a= b.

This can become:

import os, shelve, zipfile

def backup_to_zip(folder, destination, copies=1):
    f_name= os.path.basename(folder)
    with shelve.open('names') as names:
        num = names.setdafault(f_name, 1)

        os.chdir(folder)
        for num in range(num, num + copies):
            zip_path = os.path.join(destination, f'{f_name}_{num}.zip')
            with zipfile.ZipFile(zip_path, 'w') as the_zip:
                for _, _, filenames in os.walk(folder):
                    for filename in filenames:
                        the_zip.write(filename)

        names[f_name] = num

You could also use pathlib rather than os.path. Which may allow you to use:

def backup_to_zip(folder, destination, copies=1):
    folder = Path(folder)

    with shelve.open('names') as names:
        num = names.setdefault(folder.name, 1)
        path = folder / Path(destination)
        for num in range(num, num + copies):
            with zipfile.ZipFile(path / f'{folder.name}_{num}.zip', 'w') as f:
                for file in folder.iterdir():
                    if file.is_file():
                        f.write(file.name)

        names[folder.name] = num
\$\endgroup\$
6
  • \$\begingroup\$ okay so I got 4 things to point out: 1) So using with - as closes the stated file when the program ends? 2) Doesn't setdefault method adds key: value to the dictionary ONLY if the key doesn't exists? 3) B-b-but I like using camelCase... 4) I fixed the problem in my own code by changing the cwd back to the original one after the final loop ends \$\endgroup\$ – Skullz Jan 2 '18 at 10:18
  • \$\begingroup\$ @Skullz 1) it closes when you exit the with scope. Wether that be by normal execution, or via an error. 2) Yes, that's what you're doing. 3) Python programmers only use them for Classes... \$\endgroup\$ – Peilonrayz Jan 2 '18 at 10:22
  • \$\begingroup\$ hmm.. when I ran the first code you mentioned I got num = names.setdafault(f_name, 1) AttributeError: 'DbfilenameShelf' object has no attribute 'setdafault'. Doesn't that mean that shelves aren't really dictionaries in that regard? \$\endgroup\$ – Skullz Jan 2 '18 at 10:52
  • \$\begingroup\$ @Skullz that's due to a late night typo. setdafault -> setdefault \$\endgroup\$ – Peilonrayz Jan 2 '18 at 10:55
  • \$\begingroup\$ oh my. I didn't notice it, sorry about that. It partially works now, but it still doesn't solve my main issue: the shelve files .bak and .dat are being copied into the destination .zip. Plus, an error is being produced with _io.open(self._datfile, 'rb+') as f: FileNotFoundError: [Errno 2] No such file or directory: 'names.dat'. I'd like to know how to fix this, but I'll update my post later with my final code either way. Thank you for your time. \$\endgroup\$ – Skullz Jan 2 '18 at 11:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.