Find filenames with 4 digits which are not divisible by 20 and delete them

Question

I have a set of files in folder my_dir, which is not the folder where my Python script resides. Sample content of my_dir:

  foo.bar
  ba.bar
  moog_0001.h5
  moog_0007.h5
  moog_0020.h5
  moog_0027.h5
  moog_0032.h5
  moog_0039.h5
  moog_0041.h5
  moog_0053.h5
  moog_0060.h5

I need to:

• find the files with extension .h5
• for each such file, extract the 4 digits in the filename which follow the underscore _. Note: if a file has the extension .h5, its filename always contains a substring _dddd. There can be other digit groups in the full pathname of the file, but none starts with a _, followed by 4 digits, and ends with a .
• if the resulting integer is not divisble by 20, delete the corresponding file

According to these rules, after running the script the content of the folder my_dir must be:

  foo.bar
  ba.bar
  moog_0020.h5
  moog_0060.h5

My solution:

import os
import re

MY_DIR = "/tmp/logs/20190519T1032"

root, dirs, files = next(os.walk(MY_DIR, topdown=True))
files = [ os.path.join(root, f) for f in files ]
print(files)

files = [ file for file in files if file.endswith(".h5") ]

for file in files:
    match = re.search(r'_\d{4}', file).group(0)
    match = match[1:]
    digits = int(match)
    if digits % 20 != 0:
        print("remove file " + file )
        os.remove(file)
    else:
        print("skip file " + file)

Any suggestions? I was told to put the content of the for block in a function, in order to substitute the for block with a list comprehension, but I don't know if it would be significantly faster (the number of files is O(10³), tops). Also, the resulting function wouldn't do only one thing: at the very least, it would extract the 4 digits and delete those files for which the corresponding integer is not divisible by 20. I think that most functions should only one thing.

Answer 1

Requirements

The requirements are somewhat imprecise because they rely on untold assumptions about the filenames we should expect. Before implementing anything, we should try to think about the different inputs we can have and how we should handle them.

In our case, this could correspond to:

• what if the regexp does not match (no underscore or less than 3 numbers) ?
• what if we have more than 4 numbers ? Should we consider only the first 4 ?
• what if the pattern appears more than once ?

In order to tests tests, I've defined the following list of file names from my invention.

files = [
    '.h5',
    '_.h5',
    'foo_.h5',
    'foo_123.h5',
    'foo_1234.h5',
    'foo_1240.h5',
    'foo_12345.h5',
    'foo_12340.h5',
    'foo_12340.h5',
    'foo_12400.h5',
    'foo_12403.h5',
    'foo_123_bar.h5',
    'foo_1234_bar.h5',
    'foo_1240_bar.h5',
    'foo_12345_bar.h5',
    'foo_12340_bar.h5',
    'foo_12400_bar.h5',
    'foo_12403_bar.h5',
    'foo_1234_bar_1240.h5',
    'foo_1240_bar_1234.h5',
]

From here, changes in the code may be considered wrong as they could change the way the code behaves (on some of the inputs above) but I do not know what the expected behavior is.

Improving the code

The first thing we could do is try to be more robust when the pattern does not match.

Usually after a call to re.search, the next step is "if match" (or "if match is None").

Taking this chance to define variables with better names ("digits" instead of re-using "match" for the string of 4 digits, "n" instead of "digits" from the corresponding integer), we'd have something like:

for f in files:
    match = re.search(r'_\d{4}', f)
    if match is not None:
        digits = match.group(0)[1:]
        n = int(digits)
        if n % 20 != 0:
            print("remove file " + f + " (" + digits + ")")
            continue
    print("skip file " + f)

Removing the need for modulo

Division by 20 is simple enough so that the corresponding logic can be moved into the regexp.

Disclaimer: This may not correspond to something we usually want to do but it is fun and interesting so let's do it anyway :)

A number is divisible by 20 if and only if:

• last digits is a 0

• the digit before that is divisible by 2

We could write:

for f in files:
    match = re.search(r'_\d\d[02468]0', f)
    if match is None:
        print("remove file " + f)
    else:
        print("skip file " + f)

Organisation

It would indeed be worth defining small functions to make your code easier to understand. We could imagine having a function "def file_must_be_deleted(filename)" return a boolean.