2
\$\begingroup\$

I've recently started programming and have been working through some problems such as the one below:

Write a program that finds all files with a given prefix, such as spam001.txt, spam002.txt, and so on, in a single folder and locates any gaps in the numbering (such as if there is a spam001.txt and spam003.txt but no spam002.txt). Have the program rename all the later files to close this gap.

I was able to solve with below, but I can't imagine this being the most elegant solution - just seems very convoluted and overcomplicated. I would like to build good habits and train myself to write more efficient code - any suggestions for how can this be optimized/shortened?

#! python3
# fillGap.py - finds all files with given prefix in folder 
# and locates gaps in numbering and fills the gap

import os, re, shutil

# Arguments: folder, prefix
def fillGap(folder, prefix):

    # Create regex with prefix + number + extension     
    prefixRegex = re.compile(r'(%s)((\d)+)(\.[a-zA-Z0-9]+)' % prefix)

    # Make sure folder path is absolute
    folder = os.path.abspath(folder)

    # Create list with all files that match regex
    fileList = []
    for item in os.listdir(folder):
        if os.path.isfile(os.path.join(folder, item)) and prefixRegex.search(item):
            fileList.append(item)

    # Look for the starting number by incrementing, starting from 0
    Number = 0
    Completed = False
    while True:
        for filename in fileList:
            mo = prefixRegex.search(filename)
            if Number == int(mo.group(2)):
                length = len(str(mo.group(2)))
                fileList.remove(filename)
                Completed = True
                break
        Number += 1
        if Completed == True:
            break

    # Increment from starting number, finding next largest file and renaming
    nextNumber = Number
    while True:
        for filename in fileList:
            mo = prefixRegex.search(filename)
            if nextNumber == int(mo.group(2)):
                # New number including prefixed zero's
                subNumber = '0' * (length - len(str((Number)))) + str(Number)
                # New filename with updated number
                newFileName = prefixRegex.sub(r'%s%s\4' % (prefix, subNumber), filename)
                shutil.move(os.path.join(folder, filename), os.path.join(folder, newFileName))
                fileList.remove(filename)
                Number += 1
        if len(fileList) == 0:
            break   
        nextNumber += 1
\$\endgroup\$
0

3 Answers 3

5
\$\begingroup\$

This is actually a very tricky function to write well, especially for a beginner.

Bugs

Be careful whenever you do string interpolation: escaping is often needed. Here, you have plopped prefix directly into your regex, without considering whether there might be characters within the prefix that might have special significance in the regex language. Here, you need to call re.escape().

if fileList is empty, you'll get stuck in an infinite loop, since Completed will never be set to True.

You probably want to require all of the filenames to start with the prefix. Instead, you do re.search(), which performs an search that is not anchored at the beginning of the string. You should use either re.match() or include a ^ at the beginning of your regex.

You zero-pad the numbers so that they are all the same width, based on the width of the number in the smallest-numbered file. If the numbers are all the same width, then that's fine. However, if they might not all be the same width, then it makes more sense to go with the width of the largest-numbered file.

Also, it's not obvious what the right behaviour is if spam1.txt and spam001.txt both exist. Maybe that isn't a concern for you, but I thought I should mention it.

Algorithm

Your strategy works well enough for small numbers. I wouldn't recommend it for a files named like photo_2016022235959.jpg — it could take a while to count that high! Sorting might be a better strategy than counting up from 0.

The nested while True: for filename in fileList: loops are cumbersome. You can often avoid loops in Python by using list comprehensions and generator expressions. An additional benefit of using comprehensions and generator expressions is that you can "say what you mean" instead of specifying what to do step-by-step.

There is some repetition of the string-analysis code in the two loops. I recommend performing the regex match once and storing the match results instead of the filenames.

Implementation details

I don't see any reason for converting folder to an absolute path.

os.path.isfile(…) involves a system call, and is much more expensive than a regex test. Therefore, you should reorder the tests to do the quick test first:

if prefixRegex.search(item) and os.path.isfile(os.path.join(folder, item)):

However, since Python 3.5, you can use os.scandir() instead of os.listdir() if you also want information about the entries such whether it is a regular file (thus rendering the previous point moot). os.scandir() produces os.DirEntry objects as results, such that you can retrieve the entry.name of the file and test entry.is_file() efficiently.

Instead of zero-padding numbers by concatenating strings yourself…

# New number including prefixed zero's
subNumber = '0' * (length - len(str((Number)))) + str(Number)
# New filename with updated number
newFileName = prefixRegex.sub(r'%s%s\4' % (prefix, subNumber), filename)

… use str.format():

'{}{:0{width}}.{}'.format(prefix, n, extension, width=width)

That means, basically, prefix + str(n) + '.' + extension. A :05 format spec makes the number zero-padded to five digits; writing it as :0{width} makes it take the width from the parameter named width.

Counting loops are often better written using enumerate().

PEP 8 is the official Python style guide. The naming conventions say that you should use lower_case_with_underscores for function and variable names. Following some other capitalization convention is allowable if you need to interoperate with existing code, but is not recommended.

Suggested solution

import os, re, shutil

def fill_gaps(folder, prefix):
    filename_re = re.compile(r'{}(\d+)\.(.*)'.format(re.escape(prefix)))
    name = lambda match: match.group(0)
    number = lambda match: int(match.group(1))
    ext = lambda match: match.group(2)

    # A list of regular expression matches for all filenames that fit the
    # pattern, sorted numerically. filter(None, ...) eliminates false values,
    # namely the failed re.match(...) calls that returned None.
    matches = sorted(filter(None, (
        filename_re.match(entry.name)
        for entry in os.scandir(folder)
        if entry.is_file()
    )), key=number)

    if not matches: return

    first_num = number(matches[0])
    width = max(len(match.group(1)) for match in matches)

    for n, match in enumerate(matches, first_num):
        old_name = os.path.join(folder, name(match))
        new_name = os.path.join(
            folder,
            '{}{:0{width}}.{}'.format(prefix, n, ext(match), width=width)
        )
        if new_name != old_name:
            if os.path.exists(new_name):
                raise FileExistsError(new_name)
            # print('mv {} {}'.format(old_name, new_name))
            shutil.move(old_name, new_name)

I'd rather have the operation fail than risk overwriting an existing file, so I've added the test if os.path.exists(new_name): raise FileExistsError(new_name).

\$\endgroup\$
5
  • \$\begingroup\$ Incredible answer - thank you so much. I haven't encountered most of what you've written in the solution, so it's going to take some time to look up and digest. Few questions - 1. I looked upfilter() and it filters a list if the first argument returns True - what does None do in your code? 2. What is entry.name? 3. What is :0 and wouldn't n replace the entire {:0{width}}? 4. So the program will raise an error if the file already exists and just leave it with old_name? 5. Author of the book I'm reading advocates camel case - do you think underscore is better? \$\endgroup\$
    – cafekaze
    Commented Jun 23, 2016 at 6:32
  • \$\begingroup\$ Received some kind of error when trying to run the program, not sure what this means: Traceback (most recent call last): File "D:\Users\Erick\Desktop\fill_gaps.py", line 31, in <module> fill_gaps('D:\\Users\\Erick\\Desktop\\Sample Folder','spam') File "D:\Users\Erick\Desktop\fill_gaps.py", line 29, in fill_gaps shutil.move(old_name, new_name) \$\endgroup\$
    – cafekaze
    Commented Jun 23, 2016 at 6:34
  • \$\begingroup\$ Second half of errors: File "C:\Users\Erick\Python\lib\shutil.py", line 552, in move copy_function(src, real_dst) File "C:\Users\Erick\Python\lib\shutil.py", line 251, in copy2 copyfile(src, dst, follow_symlinks=follow_symlinks) File "C:\Users\Erick\Python\lib\shutil.py", line 114, in copyfile with open(src, 'rb') as fsrc: FileNotFoundError: [Errno 2] No such file or directory: 'spam001.txt' \$\endgroup\$
    – cafekaze
    Commented Jun 23, 2016 at 6:35
  • \$\begingroup\$ I've fixed a bug and added explanations in Rev 2. (Look at the revision history to see the additions.) \$\endgroup\$ Commented Jun 23, 2016 at 7:34
  • 2
    \$\begingroup\$ sorted always returns a list, so list(sorted(spam)) is redundant and creates an unnecessary copy. \$\endgroup\$ Commented Jun 23, 2016 at 15:11
3
\$\begingroup\$

Some suggestions:

  1. The glob module will make it much easier to find all files matching a given pattern. However, it still returns a list of file names.
  2. The most effective way to do this is the pathlib module, which allows you to get all files matching a given pattern and return them as objects. These can be renamed within the object.
    1. The glob method allows you to find all files matching a given pattern.
    2. The name method gets you the file name without the rest of the path.
    3. The with_name method lets you get an identical path with the name replaced with a new name.
    4. The rename method allows you to move the file to a new name.
  3. The re module is overkill if you just want to strip off a prefix, assuming that files are of the form {prefix}{num}.{ext}. You
    1. You can just use slicing to get rid of the prefix. So fname[len(prefix):]
    2. You can use the partition method to strip off all extensions, by partitioning on '.' and only taking the first result. If you know the files only have one extension, you can just use the stem method of the path.
    3. You can use replace to replace one known string (in this case the original number) with another known string (in this case the new number).
  4. It is generally recommended to follow the PEP 8 style. You are doing that pretty well, the one exception I noticed is that fillGap should be fill_gap.
  5. If you are going to put a comment just above a function, it is better to make it the function docstring by putting it in """triple quote""" right after the function declaration.
  6. Although it is a little overkill in your case, the argparse module is usually better for parsing command-line arguments (if you want to stay in the standard library, some third-party projects are better still). It has the advantage that you can set documentation and optional values (like defaulting to the current directory if none is specified).

So this is how I would do it:

#! python3
"""fillGap.py - finds all files with given prefix in folder
and locates gaps in numbering and fills the gap
"""

import argparse
from pathlib import Path


def fill_gap(prefix, folder='.'):
    """Make the numbering of files starting with a prefix consecutive.

    fill_gap(prefix, folder='.')
    Find all files of the form `{prefix}{num}.{ext}`
    and make `{num}` consecutive.

    If `folder` is defined, search in that folder,
    otherwise use the current folder.
    """
    paths = [path for path in Path(folder).glob(prefix+'*.*') if path.is_file()]
    numstrs = (path.name[len(prefix):].partition('.')[0] for path in paths)

    pairs = ((path, numstr) for path, numstr in zip(paths, numstrs)
            if numstr.isdecimal())
    pairs = sorted(pairs, key=lambda x: int(x[1]))
    if not pairs:
        return

    ndig = max(len(num) for _, num in pairs)

    for i, (path, numstr) in enumerate(pairs, int(pairs[0][1]):
        new_numstr = str(i).rjust(ndig, '0')
        new_name = path.name.replace(numstr, new_numstr)
        print(path.name, ' -> ', new_name)
        path.rename(path.with_name(new_name))


if __name__ == '__main__':
    parser = argparse.ArgumentParser(
        description="""finds all files with given prefix in folder
and locates gaps in numbering and fills the gap.""")
    parser.add_argument('prefix', help='The file prefix to match.')
    parser.add_argument('folder', help='The directory to process.',
                        default='.', nargs='?')
    args = parser.parse_args()
    fill_gap(args.prefix, args.folder)
\$\endgroup\$
3
  • \$\begingroup\$ You should use os.path.splitext(path.name)[0] instead of partition, just in case prefix happen to contains '.'. \$\endgroup\$ Commented Jun 23, 2016 at 8:18
  • \$\begingroup\$ That has the same problem as Path.stem: it only strips off the first extension. So if one of the files was spam0001.tar.gz, it would give you spam0001.tar, which isn't right. But your point is valid, so I changed it to strip off the prefix before partitioning. \$\endgroup\$ Commented Jun 23, 2016 at 14:27
  • \$\begingroup\$ I have updated my comment to handle invalid names as well, counting correctly \$\endgroup\$ Commented Jun 23, 2016 at 15:00
2
\$\begingroup\$

I extracted the code for computing the "list" of files that need to be renamed into a separate function (get_files) for clarity. I replaced you os.listdir call with os.scandir because your original code needed the file type (the os.path.isfile call). The function also saves the match of the regex for later when the file is renamed.

The regex was simplified because only the number is needed. It's also applied only on the file name instead of the full path, which should speed things up a little bit.

re.sub can be replaced with some simpler string processing, although I haven't benchmarked my new code to verify that it's really faster.

shutil.move can be replaced with the simpler os.rename since all the files are located under the same folder.

Last, but not least, instead of searching for gaps which can be costly in interpreters/high-level languages, I just rename all the files in ascending order starting from the lowest number, of course. By the way, take note that fileList.remove(filename) has a time complexity of O(N) (source).

import os, re, sys

def get_files(folder, regex):
    """ -> (match, DirEntry) iter for files matching regex"""
    for f in os.scandir(folder):
        if not f.is_file():
            continue
        m = regex.fullmatch(f.name)
        if not m:
            continue
        yield (m, f)

def fill_gap(folder, prefix):
    # regex for extracting the number
    num_regex = re.compile(r'{}(\d+)\.[^.]+'.format(re.escape(prefix)))

    # files to be renamed
    files = sorted(get_files(folder, num_regex),
                   key=lambda x: int(x[0].group(1)))

    # find out the starting number
    min_num = int(files[0][0].group(1))

    # figure out the maximum length of the number string,
    # e.g. for spam001.txt and spam2.txt it's 3
    num_len = max(len(m.group(1)) for m, f in files)
    # compute the format spec of the numbers
    fmt = '0{}'.format(num_len)

    # rename the files starting with the lowest number
    for i, (m, f) in enumerate(files, min_num):
        new_num = format(i, fmt)
        new_name = f.name[:m.start(1)] + new_num + f.name[m.end(1):]
        print(f.name, ' -> ', new_name)
        os.rename(f.path, os.path.join(folder, new_name))

fill_gap(sys.argv[1], sys.argv[2])
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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