Grouping filenames by basename and extension

Question

I have created a function that parses a directory tree, and returns a list of lists that groups specific files within for later program specific parsing. I feel like it could be done better, considering I've used dummy variables, and was hoping some of you could help.

def get_output_files(path) -> list:
    """
    This routine crawls through a directory tree, and checks for groups of job files, and groups based on:
        .sh   - run script file
        .hin  - hamiltonian creation input file
        .in   - iterate input file
        .hout - hamiltonian creation output file
        .out  - iterate output file
    to be used for parsing.

    Checks for grouping based on base name of file, if file suffix does not exist, then returns None in place.
    Example:  directory "/home/test" contains the following files:
        run.sh
        run.hin
        run2.in
        run.hout
        run2.out

    will return the following list:
        [
        [run.sh, run.hin, None, run.hout, None],
        [None, None, run2.in, None, run2.out]
        ]
    """
    jobs = []
    suffixes = ['.sh', '.hin', '.in', '.hout', '.out']
    for root, dirs, files in os.walk(path):
        for file in files:
            base, end = file.rsplit('.', 1)
            dummy_list = []
            for suffix in suffixes:
                if base + suffix in files:
                    dummy_list.append(os.path.join(root, base + suffix))
                else:
                    dummy_list.append(None)
            jobs.append(dummy_list)
    real_jobs = []
    for num, job in enumerate(jobs):
        if not all(element is None for element in job):
            real_jobs.append(job)
    return jobs

Answer 1

Don't worry about dummy variable names, naming is hard, and it can be fixed up later.

For my solution, there are a number of changes:

• code is simplified by having a short inner function

• the "yield" operator produces results one at a time, vs a list of results being processed in batch

• the original code would process job files multiple times. IE if beer.sh and beer.hin were in a directory, the job would be scanned twice.

• a simple test is added; modules added in PEP8 style; documentation; name clarification

source: mglob.py with test

'''
mglob.py -- print job files in directory

TEST USAGE:
    python -m unittest mglob
'''

import glob
import os
import unittest

def find_jobs(path):
    def find_job_paths(root, files, base):
        """
        given a job base filename, list related job files that exist
        """
        JOB_SUFFIXES = ['.sh', '.hin', '.in', '.hout', '.out']
        for suffix in JOB_SUFFIXES:
            newfile = base + suffix
            if newfile in files:
                yield os.path.join(root, newfile)
            else:
                yield None

    for root, dirs, files in os.walk(path):
        # process each file base name once
        bases = set(os.path.splitext(file_)[0]
                    for file_ in files)
        for base in bases:
            job_paths = list(find_job_paths(root, files, base))
            if any(job_paths):
                yield job_paths

class TestFind(unittest.TestCase):
    def test_simple(self):
        result = list(find_jobs('temp'))
        self.assertEquals(
            result, [
                [None, None, 'temp/run2.in', None, 'temp/run2.out'],
                ['temp/run.sh', 'temp/run.hin', None, 'temp/run.hout', None],
            ],
            result)

Answer 2

Well one way I see you could be more Pythonic is by changing the line where you have

for suffix in suffixes:
    # ....

You already split the file and stored the file type in the variable 'end' Therefore all you need to do is to check if the value stored in end is in the suffixes list

if end in suffixes:
    # ....
else:
    # ....

And another thing, I don't see where real_jobs is being useful. You only return jobs and you didn't modify it in the last few lines of code. So, unless you have a typo or something, the last set of iteration is a waste of CPU cycles

Answer 3

It looks good. I see one unnecessary call to enumerate in your last for loop as your are not using the array index num in that logic. (and as Michael mentioned, this loop didn't appear to be used)