7
\$\begingroup\$

I'm a complete amateur at python. Other than hello world programs, this is the first thing I've ever done in Python.

I cannot find a good way to make os.walk function the way I want it to so I worked on my own function for it.

I'm curious which would be the best way to do something.

import os
def atDirList(startDir, maxDepth=0, minDepth=0, curDepth=0):
    output = []
    curDir = []
    curDir = os.listdir(startDir)
    if curDepth >= minDepth:
        for item in curDir:
            fullItem = os.path.join(startDir,item)
            if os.path.isfile(fullItem):
                output.append(fullItem)
    if curDepth+1 <= maxDepth:
        for item in curDir:
            fullItem = os.path.join(startDir,item)
            if os.path.isdir(fullItem):
                output = output+atDirList(fullItem,maxDepth,minDepth,curDepth+1)
    return output

print(atDirList('/music/main'))

or

import os
def atDirList(startDir, maxDepth=0, minDepth=0, curDepth=0):
    output = []
    curDir = []
    curDir = os.listdir(startDir)
    if curDepth >= minDepth:
        for item in curDir:
            fullItem = os.path.join(startDir,item)
            if os.path.isfile(fullItem) and curDepth >= minDepth:
                output.append(fullItem)
            elif os.path.isdir(fullItem) and curDepth+1 <= maxDepth:
                output = output+atDirList(fullItem,maxDepth,minDepth,curDepth+1)
    return output

print(atDirList('/music/main'))

Basically of those, is checking the if once, and traversing the list twice best or is traversing the list once, but checking the if on each entry best?

\$\endgroup\$
15
  • \$\begingroup\$ I am not sure I follow. Why do you need minDepth? \$\endgroup\$
    – vnp
    Apr 28, 2017 at 4:07
  • \$\begingroup\$ Reading How to Ask and [mcve] may be benificial... \$\endgroup\$
    – boardrider
    Apr 28, 2017 at 9:15
  • \$\begingroup\$ @boardrider Reading about MCVE is going to be counter-productive for Code Review. We need the context here, as opposed to Stack Overflow which only cares about the actual problem. \$\endgroup\$
    – Graipher
    Apr 28, 2017 at 11:07
  • \$\begingroup\$ @vnp minDepth is due to how some of the directories are laid out. In this case it's for generating list of mp3s, and in some cases, there will be a main directory with samples, and subdirectories with the full files. \$\endgroup\$
    – Trel
    Apr 28, 2017 at 14:01
  • \$\begingroup\$ @Graipher what context do you need in this case, I'm generating directory listings (possibly later with the addition of filtering by file extension). I'm trying to figure out which is the more optimal way to do that function. \$\endgroup\$
    – Trel
    Apr 28, 2017 at 14:02

1 Answer 1

3
\$\begingroup\$

In Python you usually want to avoid having a recursive solution, because of the maximum recursion depth. Here this will probably not be a problem, because it is 1000 by default (so unless you directory hierarchy is 1000 levels deep, you are fine).

However, I would at least make your functions generators. They have the advantage that you don't need to generate the whole list in one go, but one element at a time. For this, Python 3 has even an additional nice feature, yield from. Note that you will have to call list on the result if you need the whole list at once (which negates any positive effect of using a generator), but that you can directly iterate over the output of a generator

So, your second function would become:

def atDirList(startDir, maxDepth=0, minDepth=0, curDepth=0):
    if curDepth >= minDepth:
        for item in os.listdir(startDir):
            fullItem = os.path.join(startDir, item)
            try:
                if os.path.isfile(fullItem) and curDepth >= minDepth:
                    yield fullItem
                elif os.path.isdir(fullItem) and curDepth + 1 <= maxDepth:
                    yield from atDirList(fullItem, maxDepth, minDepth, curDepth + 1)
            except OSError:
                continue

Note that I also added a try..except block to skip over files for which you don't have enough permissions to check if they are a file or directory.

As an alternative solution, I would propose to use glob, which basically shell expands a string like "/home/graipher/*/*" to a list of all files and directories matching this pattern. This has two caveats, first we still need to filter out the files from the directories (easily achieved with filter) and second, this ignores files which start with a . (hidden files).

import glob
import os


def glob_list(start, max_depth=0, min_depth=0):
    # start out at least `min_depth` levels deep
    current_dir = os.path.join(start, *"*" * min_depth)
    for depth in range(min_depth, max_depth + 1):
        # go one level deeper
        current_dir = os.path.join(current_dir, "*")
        # print(current_dir)
        yield from filter(os.path.isfile, glob.iglob(current_dir))


if __name__ == "__main__":
    for file_name in glob_list(os.path.expanduser("~"), max_depth=2, min_depth=1):
        print(file_name)

Here I changed the parameter names to conform to Python's official style-guide, PEP8, by using lower_case instead of camelCase.

The *"*" * min_depth part is probably slightly complicated, because of all the stars. Let's break it down:

An iterable (like a list or a string) multiplied with an integer is just the iterable repeated n times:

>>> "a" * 3
"aaa"
>>> min_depth = 2
>>> "*" * min_depth
"**"

A * can be used to unpack a tuple into function arguments, like in this function that takes an arbitrary number of parameters:

>>> def f(*args):
...  print(args)
... 
>>> f(*"aaa")
('a', 'a', 'a')
>>> f(*"**")
('*', '*')

Putting this together, this just joins the base path with min_depth + 1 levels of stars:

>>> os.path.join("/home/graipher", *"*" * min_depth)
'/home/graipher/*/*'

Possible bug:

After having written this alternative implementation I noticed some strange behavior of your (second, I did not check the first) function. When minDepth is greater than zero, you would not expect any files from the base directory. However, they are still included (note that you need to start with a curDepth = minDepth, otherwise your script will never run).

This is what I would write with the additional constraints of having to return a list and possibly have a simple pattern (simple enough to be expressed with wildcard characters or an additional filter function):

def glob_list(start, max_depth=0, min_depth=0, pattern="*", func=os.path.isfile):
    output = []
    for depth in range(min_depth, max_depth + 1):
        path = os.path.join(start, *("*" * depth), pattern)
        output.extend(filter(func, glob.iglob(path)))
    return output
\$\endgroup\$
8
  • \$\begingroup\$ Yeah, I noticed that bug with minDepth after I posted it. My current one has pass for lack of permissions as I ran into that while testing on Windows because I tried it on a DIR that had a system folder in it. I'm completely lost on the generator part. My next goal is going to be adding an option to match against a regex and text string to include/exclude results (don't know if that matters). However, I'm unfamiliar with python entirely, so I'm lost on the generator part completely. (Also, is glob a usable option when the output needs to be full paths including on windows?) \$\endgroup\$
    – Trel
    May 2, 2017 at 17:48
  • \$\begingroup\$ Also, the end goal of this is going to be a m3u playlist (one full path to media file per line), so I am going to need to get to the full output in the end. \$\endgroup\$
    – Trel
    May 2, 2017 at 17:53
  • \$\begingroup\$ @Trel If you need the full output, just call it as list(glob_list(start_path, max_depth=2, min_depth=1)) and the generator is consumed into a list. And yes, glob should also work on Windows, the expansion it does is just like Linux shell expansion. If you need a simple pattern, you can just add that in the string you pass, like glob.glob("/home/graipher/Music/*.mp3") would return a list of all mp3 files in (the root of) my music directory, no need to use a regex for such a simple job IMO. \$\endgroup\$
    – Graipher
    May 2, 2017 at 19:36
  • \$\begingroup\$ the regex would be for more complex filtering, and isn't something that's doable in the way you're saying. When I'm fully 100% finished, it will be required to do regex and string filtering. As far as glob, I need the output to be c:\path\to\files\file.ext not c:\\path\\to\\files\\file.ext or c:/path/to/files/file.ext I'm not seeing any way to do this with the output from glob. Am I missing something? EDIT: does it change if I loop through the glob output rather than print it directly? \$\endgroup\$
    – Trel
    May 2, 2017 at 20:44
  • \$\begingroup\$ @Trel Note that os.path.join will use the correct path separator based on the OS you are using it on and so does glob (actually just went and checked on a Windows machine, just to make sure). Regarding a complicated filtering, just write it as a function that returns True if the passed in argument is according to the desired pattern and False otherwise and give this function to the filter, instead of os.path.isfile. Does not matter if it is a regex or not inside this function. But this is outside of the scope of your question. \$\endgroup\$
    – Graipher
    May 2, 2017 at 20:54

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.