[patch] Tighten subclass check at top of db.models.base.ModelBase.__new__.

The need that led me to file the issue will be addressed if/when model inheritance is finished. Particularly, if it supports multiple abstract base classes for mixing in columns.

This is a bug in our implementation of meta-classes and respecting sub-classing. Regardless of particular use-cases we know about, we should fix it. I'll take care of it.

(In [4607]) Fixed #2363 -- Fixed subclass checking in ModelBase? to allow for mixin superclasses. Thanks phil.h.smith@gmail.com.

Checking on name == "Model" is even 'hackisher' IMHO.

I have a Model named Model.. This check doesn't take the difference between django.db.models.Model and project.app.models.Model in account and django.db.models.ForeignKey?(Model) breaks.

from django.db import models
class Measures(models.Model):
    top = models.IntegerField()
    waist = models.IntegerField()
    hip = models.IntegerField()
    def is_major_turnon():
       import random
       return random.choice((True, False)) #Mind over matter ;)

class Model(models.Model):
    name = models.CharField(maxlength=25)
    measures = models.ForeignKey(Measures)

class AmericasTopModel(models.Model):
    season = models.IntegerField()
    model = models.ForeingKey(Model)
    def program_sucks():
       return True

This is not my real app, just some witty example. My real app records devices and their makes and models.

That name check is there to prevent a chicken-and-egg problem when the definition of (django's) Model itself triggers metaclass evaluation. I don't like it either, but there needs to be a guard against the actual reference later in the line.

A better way may be to just try resolving the reference to Model in a try. If the metaclass truly is evaluating django's Model class, a NameError? will be raise, and we can know to ignore this type. (I'll put up a patch when I get home...)

Even tighter check. With test.

Replying to phil.h.smith@gmail.com:

I've uploaded a slight mod of your patch with some added testcode harnassing my situation. I don't know if "invalid_models" is the right place for such a test, but I saw some other valid models in the file.

I'm kinda leery of adding the module check: if the name check is hackish, wouldn't it be more of the same? I concede that the only cases that would defeat it are pathological.

I'm sorry about not getting a patch up yesterday; (a tapas bar viciously leaped on me, I barely escaped with my life.) I've got an (untested) one here just to show what I meant a few comments ago. (I can't run the new test (or the others) atm, but maybe Chris would give it a crack?)

now with less name checks

Merge of phil.h.smith@gmail.com cleaner code and my test.

Replying to phil.h.smith@gmail.com:

I'm kinda leery of adding the module check: if the name check is hackish, wouldn't it be more of the same? I concede that the only cases that would defeat it are pathological.

I agree, it's just what the first patch meant to say: "Is this Model which is defined elsewhere in this module?" It's arguably more explicit then catching the NameError?, but the comment solves that.

I'm sorry about not getting a patch up yesterday; (a tapas bar viciously leaped on me, I barely escaped with my life.) I've got an (untested) one here just to show what I meant a few comments ago. (I can't run the new test (or the others) atm, but maybe Chris would give it a crack?)

Tapas bars can be very verocious. I have one living around the corner, which makes my residence in it's hunting grounds. ;)

The patch tested out fine. I've merged the code with my testcode. Maybe you can add some of your mix-in code to the test too? I've never written a mix-in in Python, but could this be a start for a test?

class PurrMixin():
    """Working Cars"""
    def make_noise(self):
        return "VROOOM"

class RacketMixin():
    """Car Wrecks"""
    def make_noise(self):
        return "RRRRRR crack BANG RRRR"

class HummingMixin():
    """Fancy Hybrid Cars"""
    running_motor = None
    def make_noise(self):
        if self.running_motor == 'Combustion':
            return "VROOOM"
        else:
            return "mmmmmm"

In principle, this looks like the right solution. Thanks, guys. :-)

A shame that Python doesn't let use access the thing raising the name error more easily for tighter checking (by including it as an attribute in the NameError? class), but I don't want to start matching against the string version of the exception, so we'll leave that.

I'll check this in when I get a chance.

(In [4881]) Fixed #2363 -- Improved base class checking in ModelBase? metaclass. Thanks to combined work from phil.h.smith@gmail.com and Chris.Wesseling@cwi.nl.