Scanning a directory for plugins and loading them

I'm working on a simple dictionary tool, with a base class that can be extended by plugins to represent different dictionaries. The plugins are organized in the filesystem like this:

plugins/
├── dict1
│   ├── ...
│   └── dict1.py
└── dict2
    ├── ...
    └── dict2.py

Each plugin defines a class named Dictionary, which happens to extend a BaseDictionary, in case that's relevant.

I implemented discovering and loading the plugins like this:

import os
from imp import find_module, load_module

BASE_DIR = os.path.dirname(__file__)
PLUGINS_PATH = os.path.join(BASE_DIR, 'plugins')

def discover_dictionaries():
    import plugins
    for plugin_name in os.listdir(PLUGINS_PATH):
        plugin_path = os.path.join(PLUGINS_PATH, plugin_name, plugin_name + '.py')
        if os.path.isfile(plugin_path):
            try:
                fp, pathname, description = find_module(plugin_name, plugins.__path__)
                m1 = load_module(plugin_name, fp, pathname, description)
                fp, pathname, description = find_module(plugin_name, m1.__path__)
                m2 = load_module(plugin_name, fp, pathname, description)
                class_ = getattr(m2, 'Dictionary')
                yield plugin_name, class_()
            except ImportError:
                print('Error: could not import Dictionary from {0}'.format(plugin_path))

This works, but it's kind of ugly, and probably I'm doing it all wrong. How to do this the "right way," or at least better?

If you want to play with the code to see if your refactoring ideas work, clone the project from GitHub, and run this as a sanity check:

# should output: dummy
python -c 'import util; print util.discover_dictionaries().next()[0]'

I started a project with a structure similar to the one you detailed:

My first order of business was to move the import plugins out of the discover_dictionaries() function and to the top of the file.

My second order of business was to create an iterator which would iterate over the discover_dictionaries() generator and print what it had found:

for next_plugin in discover_dictionaries():
    print next_plugin

My third order of business was to create empty classes in each of the dict* paths, which looked similar to:

import os

class Dictionary:
    def __init__(self):
        pass

Following that, I added a print to the generator to see what it saw:

def discover_dictionaries():
    for plugin_name in os.listdir(PLUGINS_PATH):
        plugin_path = os.path.join(PLUGINS_PATH, plugin_name, plugin_name + '.py')
        print plugin_path

This gave me the following output:

C:/Users/jsanc623/PycharmProjects/AutoloadModules\plugins\dict1\dict1.py
C:/Users/jsanc623/PycharmProjects/AutoloadModules\plugins\dict2\dict2.py
C:/Users/jsanc623/PycharmProjects/AutoloadModules\plugins\dict3\dict3.py
C:/Users/jsanc623/PycharmProjects/AutoloadModules\plugins\__init__.py\__init__.py.py
C:/Users/jsanc623/PycharmProjects/AutoloadModules\plugins\__init__.pyc\__init__.pyc.py


If I put the same print statement inside the if os.path.isfile(plugin_path): block, the output was:

('dict1', )
('dict2', )
('dict3', )


So, it does work. Now, let's see how we can clean it up.

The first thing I did was to get rid of BASE_DIR:

BASE_DIR = os.path.dirname(__file__)
PLUGINS_PATH = os.path.join(BASE_DIR, 'plugins')


as follows:

PLUGINS_PATH = os.path.join(os.path.dirname(__file__), 'plugins')


Next, we can get rid of class_ by replacing the yield statement from:

class_ = getattr(m2, 'Dictionary')
yield plugin_name, class_()


To a much simpler:

yield plugin_name, getattr(m2, 'Dictionary')()


Next, I added an if statement to return if it encountered a 'pyc' or 'init' file:

for plugin_name in os.listdir(PLUGINS_PATH):
if 'pyc' in plugin_name or 'init' in plugin_name:
return
else:


We can also get rid of m1, by casting pathname:

fp, pathname, description = find_module(plugin_name, plugins.__path__)
fp, pathname, description = find_module(plugin_name, [pathname])
m2 = load_module(plugin_name, fp, pathname, description)


But, we can get even sneakier and get rid of the assignment on the first find_module by doing so:

fp, pt, dc = find_module(plugin_name, [find_module(plugin_name, plugins.__path__)[1]])
m2 = load_module(plugin_name, fp, pt, dc)


Which, granted makes our lines a little long. However, let's get a little crazier with our yield:

yield plugin_name, getattr(load_module(plugin_name, fp, pt, dc), 'Dictionary')()


So, in the end, the original would now look like:

import os
from imp import find_module, load_module
import plugins

PLUGINS_PATH = os.path.join(os.path.dirname(__file__), 'plugins')

def discover_dictionaries():
for plugin_name in os.listdir(PLUGINS_PATH):
if 'pyc' in plugin_name or 'init' in plugin_name:
return
else:
plugin_path = os.path.join(PLUGINS_PATH, plugin_name, plugin_name + '.py')
if os.path.isfile(plugin_path):
try:
f, p, d = find_module(plugin_name, [find_module(plugin_name, plugins.__path__)[1]])
yield plugin_name, getattr(load_module(plugin_name, f, p, d), 'Dictionary')()
except ImportError:
print('Error: could not import Dictionary from {0}'.format(plugin_path))


And it would still function. If you want to make it a bit more concise at the cost of iterating over 'pyc' and 'init' files, you can omit the if/else statement:

import os
from imp import find_module, load_module
import plugins

PLUGINS_PATH = os.path.join(os.path.dirname(__file__), 'plugins')

def discover_dictionaries():
for plugin_name in os.listdir(PLUGINS_PATH):
plugin_path = os.path.join(PLUGINS_PATH, plugin_name, plugin_name + '.py')
if os.path.isfile(plugin_path):
try:
f, p, d = find_module(plugin_name, [find_module(plugin_name, plugins.__path__)[1]])
yield plugin_name, getattr(load_module(plugin_name, f, p, d), 'Dictionary')()
except ImportError:
print('Error: could not import Dictionary from {0}'.format(plugin_path))


Readability out of the window, but we save assignments?

StackExchange Code Review Q#59715, answer score: 4