db.models.base.ModelBase.__new__ fails to deduce app_label with model packages

[bjorn@…]

In the latest magic-removal branch, if models are in a separate directory (package) instead of just a module they will have one more component in their path.

db.models.base.ModelBase.__new__ tries to deduce the app_label from the model's module, but fails in this case. It will call register_module() with the parent directory as its app_label, but in this case the parent directory is the model package itself, resulting in it registering the module wrongly. The code in question is:

        if getattr(new_class._meta, 'app_label', None) is None:
            # Figure out the app_label by looking one level up.
            # For 'django.contrib.sites.models', this would be 'sites'.
            new_class._meta.app_label = model_module.__name__.split('.')[-2]

For example, a model in myproj/apps/myapp/models/mymodel.py will register itself as belonging to myproj.apps.myapp.models with app_label models.

-- Bjorn

[Adrian Holovaty]

#1686 was a duplicate.

[erob@…]

Hi,

I'm getting this problem too when unit testing django with unittest.

Here's the code in question:

from django.conf import settings

import os, os.path
import unittest

class DjangoTest(unittest.TestCase):
      def setUp(self):
          # Test fixture method
          # Must configure django settings first
          try:
              settings.configure(DEBUG=1,
              DATABASE_NAME=":memory:",
              DATABASE_ENGINE="sqlite3", 
              TEMPLATE_DEBUG=1,
              TEMPLATE_DIRS=(os.getcwd())) # current directory
              #self.settings = settings
              reload(settings)
          except Exception, e:
              import warnings
              warnings.warn(e, UserWarning, stacklevel=2)
              pass
          self.settings = settings
         
      def tearDown(self):
          # test closure method
          self.settings = None

      def testSettings(self):
          # put some code here
          self.failIf(self.settings.DATABASE_ENGINE != "sqlite3",
self.settings.DATABASE_ENGINE)

      def testApp(self):
          from django.db import models
          class App(models.Model):
                pass

if __name__ == '__main__':
   unittest.main()

The traceback msg:

% python test_models.py
/usr/local/lib/python2.4/unittest.py:251: UserWarning: reload() argument must be module
  self.setUp()
E/usr/local/lib/python2.4/unittest.py:251: UserWarning: Settings already configured.
  self.setUp()
.
======================================================================
ERROR: testApp (__main__.DjangoTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_models.py", line 48, in testApp
    class App(models.Model):
  File "/usr/local/lib/python2.4/site-packages/Django-0.95-py2.4.egg/django/db/models/base.py", line 45, in __new__
    new_class._meta.app_label = model_module.__name__.split('.')[-2]
IndexError: list index out of range

----------------------------------------------------------------------
Ran 2 tests in 0.396s

FAILED (errors=1)

[erob@…]

is there a known work-around for this ??

[anonymous]

Here is partial work-around, which works with os.getcwd and os.path.basename.
Its sufficient for passing a really simple unit test but comments would be of
courses welcome.

Index: base.py
===================================================================
--- base.py     (revision 3350)
+++ base.py     (working copy)
@@ -42,7 +42,13 @@
         if getattr(new_class._meta, 'app_label', None) is None:
             # Figure out the app_label by looking one level up.
             # For 'django.contrib.sites.models', this would be 'sites'.
-            new_class._meta.app_label = model_module.__name__.split('.')[-2]
+            try:
+               new_class._meta.app_label = model_module.__name__.split('.')[-2]
+           except IndexError, e:
+                cwd = os.getcwd()
+               new_class._meta.app_label = os.path.basename(cwd)
+               print new_class._meta.app_label
+

         # Bail out early if we have already created this class.
         m = get_model(new_class._meta.app_label, name)

[erob@…]

patch for this issue -- is this working?

[erob@…]

needs some feedback/inputs on this patch..

[Malcolm Tredinnick]

A quick review of this patch. It has a few problems:

1. os.getcwd() can't be right: the result returned by that depends on the current working directory, not the import path of the model. We don't change directories at any point during the import process, so the results returned here will be dependent upon factors completely outside Django's control.

2. It is not a full solution to the problem because it only works for a single hierarchy level (it looks like an import of models/foo/bar.py will still fail). We should try to get something that works for the general case.

3. some very minor points: the indentation is inconsistent and there is a print statement in there. If you're not going to use the result of the exception, there's no point catching "e".

[erob@…]

mtredinnick: Thanks for the patch review.
I've made a updated diff but I cant upload..

I'm not sure what you mean by 'single hierarchhy level', can u please elaborate on that?
However, for simple unit testing with unittest things seems ok here.

[Malcolm Tredinnick]

My second point in my previous comment was talking about an application structure that looked like this:

application/
   __init__.py
   models/
      __init__.py
      foo/
         __init__.py
         bar.py
      baz/
         __init__.py
         blam.py

So you would be importing models as

from application.models.foo.bar import MyModel

and so on. Does the change still work then?

[erob@…]

I have modified the test case in order to reproduce
the application structure/environment as you suggested.

The test still works, so now I don't know how to break it ;)

Here's the app structure:

models/
 test_models.py <-- the test case
 foo/
    __init__.py
    bar/
    __init__.py
    baz.py 
}}

In baz.py:
{{{
#!python
from django.db import models
}}} 


Here's the test-case:

{{{
#! /usr/bin/env python

__ident__ = "$Id: test_models.py 48 2006-07-04 17:07:37Z erob $"

from test.test_support import run_unittest, verbose
from django.conf import settings

import os, os.path
import unittest

class DjangoTest(unittest.TestCase):
      def setUp(self):
          # Test fixture method
          # Must configure django settings first
          try:
              settings.configure(DEBUG=1,
              DATABASE_NAME=":memory:",
              DATABASE_ENGINE="sqlite3", 
              TEMPLATE_DEBUG=1,
              TEMPLATE_DIRS=(os.getcwd())) # current directory
              #self.settings = settings
              #reload(settings)
          except Exception, e:
              import warnings
              warnings.warn(e, UserWarning, stacklevel=2)

          if settings is not None:
             self.settings = settings
             
          try:
              from foo.bar.baz import models
          finally:
              self.models = models
         
      def tearDown(self):
          # test closure method
          self.settings = None

      def testSettings(self):
          # put some code here
          self.failIf(self.settings.DATABASE_ENGINE != "sqlite3",
self.settings.DATABASE_ENGINE)

      def testApp(self):
          # this requires a patch to base.py
          # in order to make this happen...
          models = self.models
          class BaseApp(models.Model):
                name = models.CharField(maxlength=200)
                def __str__(self):
                    return self.name
          class App(BaseApp):
                class Admin:
                      pass
          app = App()
          app.name = "Hello, World!"
          self.failUnless(isinstance(app, App), app.__module__)
          self.assertEqual(app.name, "Hello, World!", app.name)

def test_main():
    tests = [ DjangoTest ]
    run_unittest(*tests)

if __name__ == '__main__':
   test_main()
}}}

The test results:
{{{
% python test_models.py
testApp (__main__.DjangoTest) ... ok
/usr/local/lib/python2.4/unittest.py:251: UserWarning: Settings already configured.
  self.setUp()
testSettings (__main__.DjangoTest) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.606s

OK
}}}

[anonymous]

This test case does not test the problem reported in the ticket. You are not trying to load any actual models from baz.py and your test is inside your model framework, which can potentially change things.

Further, you have not correct the main problem I pointed out: any patch that requires a call to os.getcwd() to work is not a portable solution to this problem.

[Malcolm Tredinnick]

(That last comment was from me, Malcolm, again. Accidently not logged in.)

[erob@…]

updated test case

[erob@…]

Ok, I've updated the test case, but it just highlights several things
that seems incorrect in Django. I havent changed anything so far in the
Django code so its only good for diagnosis of problems.

As for the actual problem described here, I've managed to reproduce it
twice, but its far from being resolved (design bug??).

Moreover, the unit test also grab several design flaws similar to the
original issue, theses problems seems to happens more django/db/models/base.py
and django/db/models/loading.py.

For using the test case in your own project, replace the 'notmm.models' occurances
with your own model package.

[erob@…]

maybe its a good idea to use pkg_resources here.
pkg_resources.resource_string and pkg_resouces.Requirement
may be used to 'deduce' the package location. Is it
possible to distribute packages (django projects or apps)
in Eggs format ?

Example:

import pkg_resources
_app_label = pkg_resources.resource_string(module_path, module)
...

[erob@…]

update on patch above and should works for getting the app_label object.

from pkg_resources import resource_string, Requirement
model_module = 'foo/bar/models.py'
project_name = 'quux'
requirement = Requirement.parse(project_name)
app_label = resource_string(requirement, module_name)
# do stuff with app_label here

[Gary Wilson <gary.wilson@…>]

Joseph is working on fixing the app loading code and plans on opening a new ticket for it.

[jkocherhans]

Replying to Gary Wilson <gary.wilson@gmail.com>:

Joseph is working on fixing the app loading code and plans on opening a new ticket for it.

Ticket opened. #3591

[James Bennett]

Closing in favor of #3591.