Proposal for improved Mixin functionality

[pmav99]

trafficstars

I don't know if you have already considered something like this, but from an organizational point of view, I think that it would be nice if we could use use separate classes for the configuration of each django application. For example:

class DjangoDebugToolbar(object):
    pass

class DjangoAvatar(object):
    pass

class MyAppConfig(Configuration, DjangoAvatar):
    pass

class MyDevConfig(MyAppConfig, DjangoDebugToolbar):
    pass

The problem with this approach, is that the current implementation of mixins does not work with settings like the INSTALLED_APPS which are tuples (i.e. the settings get overwritten).

What I propose is to make changes in the ConfigurationBase metaclass in order to allow tuples to be concatenated instead of overwritten. I.e to allow something like this:

from configurations import Configuration

class A(object):
    A = ("A.A1", "A.A2")
    B = "A.B"

class B(object):
    A = ("B.A1", "B.A2")
    B = "B.B"
    C = "B.C"
    D = "B.D"

class Foo(Configuration, A, B):
    A = ("Foo.A1", "Foo.A2",)
    C = "Foo.C"
    E  = "Foo.E"

assert Foo.A == ("Foo.A1", "Foo.A2", "A.A1", "A.A2", "B.A1", "B.A2")
assert Foo.B == "A.B"
assert Foo.C == "Foo.C"
assert Foo.D == "B.D"
assert Foo.E == "Foo.E"

As a (simple) proof of concept, I came up with the following. It is not extensively tested but it seems to work; at least the tests continue to work :)

-        if parents:
-            for base in bases[::-1]:
-                settings_vars.update(uppercase_attributes(base))
+        if parents:
+            for base in bases:
+                for key, value in uppercase_attributes(base).items():
+                    if key in attrs:
+                        if isinstance(value, tuple):
+                            attrs[key] += value
+                    else:
+                        attrs[key] = value

Would you be interested in something like that?

[arnuschky]

This is exactly what I need. I fiddle around mixins and INSTALLED_APPS for the last hour, and this solves the problem very cleanly.

Thanks @pmav99 for contributing. I would be happy to see this merged.

[arnuschky]

Hmm, it seems that I was a bit hasty. This merges everything and does not allows to override. For example, I would like to override FIXTURE_DIRS in a subclass, which isn't possible with this solution.

Is there no way of overriding/merging selectively?

[arnuschky]

So, I came up basically with this helper function:

def merge_parent_tuples(object_, attr_, tuple_ = ()):
    """
    Helper function that merges an specific attribute from all parents of a class.
    Allows to add locally some more values.

    Define the following property if you want to merge 'INSTALLED_APPS' 
    of all parents and add some values locally:

    @property
    def INSTALLED_APPS(self):
        return merge_parent_tuples(self, 'INSTALLED_APPS', ( 'value1', 'value2' ))


    This does the same without adding furher values:

    @property
    def INSTALLED_APPS(self):
        return merge_parent_tuples(self, 'INSTALLED_APPS')

    """
    for base in object_.__class__.__bases__:
        if hasattr(base, attr_):
            tuple_ += getattr(base(), attr_)
    return tuple_

This helper allows you to selectively merge on multi-inheritance. Example:

class App1Mixin(object):
    INSTALLED_APPS = ( 'apps.app1', )


class App2Mixin(object):
    INSTALLED_APPS = ( 'apps.app2', )


class S3Mixin(object):
    # some S3 config here

    INSTALLED_APPS = ( 'storages', )


class Base( Configuration ):
    # all your config here

    # this merges the locally defined tuple with the ones of all parents, here just 'Configuration'
    @property
    def INSTALLED_APPS(self):
        return merge_parent_tuples(self, 'INSTALLED_APPS', (
            'django.contrib.admin',
            'django.contrib.auth',
            'django.contrib.contenttypes',
            'django.contrib.sessions',
            'django.contrib.messages',
            'django.contrib.staticfiles',
            'django_extensions',
            #  ....
        ))


class Development( App1Mixin, App2Mixin, Base ):
    DEBUG = True

    # add additional dev apps
    @property
    def INSTALLED_APPS(self):
        return merge_parent_tuples(self, 'INSTALLED_APPS', (
                'django_nose',
            ))


class MyPersonalDevConfig( S3Mixin, Development):
    # merge INSTALLED_APPS explicitly, but do not add anything local
    @property
    def INSTALLED_APPS(self):
        return merge_parent_tuples(self, 'INSTALLED_APPS')

This solved my problem nicely. To employ for other settings, simply change name of attribute, eg.:

class Testing( Development ):
    # add fixtures for testing
    @property
    def FIXTURE_DIRS(self):
        return merge_parent_tuples(self, 'FIXTURE_DIRS', (
                os.path.join(Base.BASE_DIR, 'some/test_fixture_dir'),
            ))

[RonnyPfannschmidt]

imho this is pretty much the wrong way around

it should look more like

Class MyMixin(Configuration):
  INSTALLED_APPS = values.MergeMixinsValue(values.BackendValue(['myapp.partapp']))

also something like

class Test(Dev):
  databases = values.AddPrefixToBase('_test')

is thinkable

[bittner]

See #217 for a related use case.