2
\$\begingroup\$

I wrote a function to find a specific file, or all the executable files with a given name and accessing flag, and I want to make sure it is as cross-platform as possible. Currently, I'm using the PATHEXT environment variable to get the extensions for files that the operating system considers executable.

import os

class CytherError(Exception):
    def __init__(self, message):
        Exception.__init__(self, message)
        self.message = 'CytherError: {}'.format(repr(message))

def where(name, flags=os.F_OK):
    result = []
    extensions = os.environ.get('PATHEXT', '').split(os.pathsep)
    if not extensions:
        raise CytherError("The 'PATHEXT' environment variable doesn't exist")

    paths = os.environ.get('PATH', '').split(os.pathsep)
    if not paths:
        raise CytherError("The 'PATH' environment variable doesn't exist")

    for path in paths:
        path = os.path.join(path, name)
        if os.access(path, flags):
            result.append(os.path.normpath(path))
        for ext in extensions:
            whole = path + ext
            if os.access(whole, flags):
                result.append(os.path.normpath(whole))
    return result

Usage:

In[-]: where('python')

Out[-]: "C:\Python35\python.exe"

Please be brutal in pointing out anything you find, except for:

  1. Commenting
  2. Docstrings
  3. PEP 8 spacing

I will have these things covered later.

\$\endgroup\$
4
\$\begingroup\$

I could have just used the function which(), offered by shutil. Documented here.

Example:

>>> from shutil import which
>>> which('python')
'C:\Python35\python.exe'
\$\endgroup\$
  • \$\begingroup\$ In my setup (Cygwin), shutil.which doesn't return anything for files the system doesn't consider executable, e.g. .py scripts. This function you've written does; I have uses for the function as you've written it. \$\endgroup\$ – r_alex_hall May 1 '18 at 10:46
2
\$\begingroup\$
  • I don't know why you even use extensions. If it isn't executable according to os, why bother checking the extensions?
  • You search only the directories that are directly in $PATH, but not the sub-directories.
  • You can use os.defpath to find the executable paths on the system instead of relying on environment variables.

  • Your code always returns copies. For example, where('python') on my computer returns ['/usr/bin/python', '/usr/bin/python']. I believe that is because you didn't continue in if os.access(whole, flags):. Since you didn't continue, that would mean that one file could match for both executability and its file extension.

My modified version of your code is as follows:

def where(name, flags=os.F_OK):
    result = []
    paths = os.defpath.split(os.pathsep)
    for outerpath in paths:
        for innerpath, _, _ in os.walk(outerpath):
            path = os.path.join(innerpath, name)
            if os.access(path, flags):
                result.append(os.path.normpath(path))
    return result

As for your exception class, you really don't need the __init__ method at all. The Exception class will take care of the message. In fact, modifying self.message does not effect how the exception is displayed. You whole class could look like this:

class CytherError(Exception):
    pass
\$\endgroup\$
  • \$\begingroup\$ According to this issue: bugs.python.org/issue5717, os.defpath does not work on windows. I also don't need to search the subdirectories. I did take your suggestion on changing CytherError and adding the continue. \$\endgroup\$ – Nick Pandolfi Mar 27 '16 at 16:08
  • \$\begingroup\$ Also, in windows, os.X_OK will return if the file is openable, not executable. For example, os.access('D:/Python35/README.txt', os.X_OK) will return True.Your following code returns an empty list to me. \$\endgroup\$ – Nick Pandolfi Mar 27 '16 at 16:13
  • \$\begingroup\$ Nick, I don't see any continue statement in your edit. Where should the continue statement be inserted? \$\endgroup\$ – r_alex_hall May 1 '18 at 10:44

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.