Good practice with conditional imports

Question

I have a module for configuration, projectConfig, to parse a sample sheet for a project:

class SampleSheetFields():
    FIELD_1 = "field1"
    FIELD_2 = "field2"


class SampleSheetFieldsOld():
    FIELD_1 = "field_1"
    FIELD_2 = "field_2"

I had been using the first class in other modules like this:

from projectConfig import SampleSheetFields as ssFields

class SomeClass

    def __init__(self):
        ...
        check(someContent, ssFields.FIELD_1)

The thing is I had developed my software using the reference to ssFields quite a lot. At some point new specifications said that the software should also use sample sheets with different field names. The quickest way I found to achieve that, without messing too much with the code, was to add the class SampleSheetFieldsOld in the projectConfig and to make a conditional import in my modules:

class SomeClass:

    def __init__(self, useOld):
        if useOld:
            from projectConfig import SampleSheetFieldsOld as ssFields
        else:
            from projectConfig import SampleSheetFields as ssFields

        ...
        check(someContent, ssFields.FIELD_1)

Note that the mandatory fields which are used have the same name so there is no conflicting or missing field. The program works as expected.

My questions are:

How bad is this practice, if it is bad; and
How can I circumvent it to make a better and more sustainable code?

So you're just using the classes for their attributes? Or do they have actual functionality, too? Have you considered making them instances, or creating a class factory? — jonrsharpe, Nov 18 '15 at 12:07
Their purpose is only to get the names, for matching purpose. — kaligne, Nov 18 '15 at 12:08

score 4 · Accepted Answer · answered Nov 18 '15 at 12:19

It's probably not the worst thing, but what I do find kind of problematic is the fact that you're now locked into two configuration options, old and new. What if you need to add a third or fourth etc. set someday? You won't be able to use a simple boolean test anymore.

Besides, it looks like your configuration options are all just simple string values, accessible via string keys. You don't need a class for that.

My suggestion is to forget about doing your configuration with the source code and use a configuration file. In your projectConfig you can have a dict which you initialize from a file, whose path/name can be provided on the command line or in whatever way is convenient. So projectConfig.py might go something like this:

config_options = {}

def load_configuration(filename):
    with open(filename) as f:
        for line in f:
            # get key and value
            config_options[key] = value

Then everywhere you need to get a field name, just access projectConfig.config_options['field_key'], e.g.

from projectConfig import config_options

class SomeClass

    def __init__(self):
        ...
        check(someContent, config_options['FIELD_1'])

Or use dict.get(key, default) if there is a reasonable default value. This way, each time you need to switch to a different set of field names, you just create a new configuration file, and you don't have to touch the code.

Python's standard library includes a configparser module which can handle the loading for you.

In the likely event that the OP doesn't really need a class, this is even easier than creating them dynamically. — jonrsharpe, Nov 18 '15 at 12:36

jonrsharpe · Answer 2 · 2015-11-18T12:17:55.240

If you only need the class attributes, you could create a class factory, using type to create new classes, like:

FIELDS = dict(
    new=dict(FIELD_1="field1", FIELD_2="field2"),
    old=dict(FIELD_1="field_1", FIELD_2="field_2"),
}


def sample_sheet_field_factory(field_spec='new'):
    return type("SampleSheetFields", (object,), FIELDS[field_spec])

This can easily be extended to further sets of field specifications, and doesn't require a conditional import:

from wherever import sample_sheet_field_factory

class SomeClass(object):

    def __init__(self, use_old):
        ss_fields = sample_sheet_field_factory("old" if use_old else "new")
        check(some_content, ss_fields.FIELD_1)

You could also use a namedtuple, rather than a class, which would be a little more lightweight. Note edits for compliance with the style guide.

Good practice with conditional imports

2 Answers2