add support for custom app_label and verbose_name

A couple of internal tests are still broken, but admin, manage.py, etc. work

A couple of questions from an initial quick read:

1. In management.py, why is the import change on line 366 (of the original) necessary? I think there's some subtlety escaping me there (if it's not necessary, it's probably worth removing so that we can tell the real changes from the stylistic ones).
2. (this question left blank, because it was stupid and I worked out I was missing something obvious.)
3. Do you really mean you have to import all models in an app's __init__. py (a big change and not ideal, but maybe inavoidable; I realise this is a bit of a tarpit) or just in app_name/models/__init__.py? The stuff in loading.py make me suspect you might mean the latter, but I might be reading it incorrectly.

Replying to mtredinnick:

A couple of questions from an initial quick read:

Hey Malcolm

First of all, thanks for taking a look at this, and second, none of this is meant to be final. This is the "get it working" version that I more or less only put up as a reference for another ticket that this fixes.

1. In management.py, why is the import change on line 366 (of the original) necessary? I think there's some subtlety escaping me

there (if it's not necessary, it's probably worth removing so that we can tell the real changes from the stylistic ones).

It has an effect. Changing the current import from
from django.db.models import get_models
from django.db.models import get_app, get_models to
would get the right effect.

3. Do you really mean you have to import all models in an app's __init__. py (a big change and not ideal, but maybe inavoidable; I realise this is a bit of a tarpit) or just in app_name/models/__init__.py? The stuff in loading.py make me suspect you might mean the latter, but I might be reading it incorrectly.

It's the latter. That could probably be worked around as well, but I haven't spent much time thinking about that part of it yet.

It looks like we have a little bit of end-goal overlap with #4144. The patch over there is just a simple move of a single line, but it relates to how Django determines app_label, so it looks like that might be best handled in this ticket.

Essentially, this patch moves the model_module.__name__.split('.')[-2] into get_app_label, and in fact seems to remove any need for sys.modules at all (unless I'm reading something wrong). If the sys.modules line referenced in #4144 were to be removed as part of this ticket, #4144 would be unnecessary, and I'd be very happy.

A new, more comprehensive (IMHO) patch

I've added an updated patch. This covers changes to make-messages.py and also documentation changes, and is against trunk revision r5146. All tests in runtests.py pass, plus browsing data from apps with a custom app label in admin and databrowse works fine. Here are the relevant changes to my settings.py:

from django.conf import app

# ...

INSTALLED_APPS = (
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.sites',
    'django.contrib.humanize',
    'django.contrib.admin',
    'django.contrib.databrowse',
    'mysite.myapp',
    'mysite.registration',
    app('mysite.orghier', 'oh', 'Organization Hierarchy'),
)

Marty: Although my latest patch still leaves the line where it is, the model_module is not being used at all. I have removed the line entirely from my working copy - once I get other feedback on the patch, I will post another patch which incorporates the feedback.

Minor tweaks (tidy-ups) to the last patch.

Minor tweaks (tidy-ups) to the last patch.

Updated patch to work cleanly against trunk revision r5171.

Replaces previous patch which was not created using svn diff - sorry.

An updated patch to cater for recent changes in trunk.

I'm seriously +1 on this -- at work we're hoping we'll be using a mix of our custom apps, apps built by other sites in our division and some of the stellar open source apps being built.

I think the modularity/plug-n-customize model of Django's "app" system is one of it's great features!

I'm recording this idea here: What if INSTALLED_APPS itself was a *class*, rather than a tuple? That would solve a lot of the wacky issues.

This patch is quite interesting.. ;-)

Would it make sense to use a dictionary and named arguments for making things more explicit?

In example:

INSTALLED_APPS = (
 app('apps.foobar', {'verbose_name': 'foobar app',
                     'app_label': 'foobar1'}),
)

#2982 was a duplicate.

#3343 needs to be dealt with when a resolution is reached here.

#4470 was a duplicate.

Replying to ubernostrum:

#4470 was a duplicate.

That ticket would have fixed the problem of app_label being required when using the Django ORM outside of an actual Django application. I often write programs at my job which use the Django ORM for non-website programming, but I always have to add the Meta class to the model definition, which is ugly and looks like black magic to my co-workers who aren't familiar with Django internals:

class Something(models.Model):
    class Meta:
        app_label = ""
    
    name        = models.CharField(maxlength=100)
    description = models.TextField()
    quantity    = models.IntegerField()

This ticket doesn't seem to address this problem, whereas ticket #4470 did.

Eli, app_label is required to cleanly split models up into multiple files inside a models module. And I'm doing my best to keep all of the app_label-related stuff here in this ticket so we can get a clean solution that solves all of the associated issues.

That makes sense. I just hope that whatever that clean solution is, it will allow us to define models outside of Django webapps without having to declare the Meta class and manually set app_label.

I'll try to find time this week to take a look at the work on this ticket so far and see how well that solves this problem, and offer any advice/patches I can come up with.

The latest patch doesn't work against the current version of the Django trunk in svn. Nor does it work against r5171, which is the revision that patch 4 is said to work against, nor does it work against r5343, which was the latest revision on the date that the most recent patch was uploaded.

Someone needs to update the patch to work with the current version of Django. I won't have time to do this for at least the next few weeks, and I'm probably the wrong person to do it anyway, since I only use the Django ORM and don't write Django webapps. However, if no one else gets to this by the time I have some more free time, I'll take a stab at it.

I'm surprised that the last patch doesn't version with the earlier SVN version of Django - before uploading the patch, I had checked that all tests passed. Never mind - I'll try to look at this over the next week or two.

Oops, that should be "... last patch doesn't work ..."

An updated patch to cater for recent changes in trunk. Applies to r6453.

Latest patch (against r6453) passes all tests (tests/runtests.py).

updated to apply cleanly against r6635

I tested this patch and updated to apply to current trunk (r6635). All works for me.

However, I do have a question why get_installed_app_paths exists? Why can't it be done in __init__ of Settings when it expands out .* app paths? That way there isn't a need to change all instances of settings.INSTALLED_APPS. Making this completely backward compatible.

Good call - I'd done it the other way because it leaves INSTALLED_APPS untouched, and I wanted the impact of the patch to be clearly visible at least until the overall idea was approved. However, though it's been quite a long time - 8 months - since my first patch, and though there have been no adverse comments and a few comments with the gist "it just works", the devs have not seen fit to pronounce on it - so I'm treading water until I get a pronouncement about it. I just occasionally check to see if trunk changes break the patch, and update the patch accordingly. I'll merge your changes into mine so that my next patch has your changes, too.

Replying to brosner:

However, I do have a question why get_installed_app_paths exists? Why can't it be done in __init__ of Settings when it expands out .* app paths? That way there isn't a need to change all instances of settings.INSTALLED_APPS. Making this completely backward compatible.

I had another look at the get_installed_app_paths vs. INSTALLED_APPS issue. With my patch, INSTALLED_APPS can contain either package names (explicit or wild-card) or app instances. However, get_installed_app_paths always returns a set of strings - the package paths of the applications. This is used, as you've seen, in a lot of places. If a user puts an app instance into INSTALLED_APPS, I'm not sure they'd take kindly to having it automatically replaced with the corresponding path string. So, get_installed_app_paths insulates the rest of the code from having to know whether the INSTALLED_APPS entries are path strings or app instances. It seems to me that some kind of encapsulation will be needed - and get_installed_app_paths performs this function. I would like to be able to use INSTALLED_APPS and do away with get_installed_app_paths - but I'm not quite sure how, yet. If you provide a patch which sorts out this issue, I'll happy incorporate it, as I mentioned.

In your testing, did you have any app instances in your INSTALLED_APPS? I'd be interested in seeing what your INSTALLED_APPS looks like. From a quick inspection of your patch, I would expect some tests to fail if INSTALLED_APPS contained any app instances, because in some places where framework code expects a string, it would get an app instance.

is #6080 not a partly shortcut?

Updated to apply cleanly against r6920.

Replying to wolfram:

is #6080 not a partly shortcut?

No, I don't believe #6080 overlaps with this ticket. That ticket is to do with loading apps from eggs; this ticket allows for easy specification of an app_label to disambiguate apps which end in the same name (e.g. 'django.contrib.admin' clashing with 'myapp.mypackage.admin'), and allows verbose names with i18n support for use in the admin (e.g. 'Authentication/Authorization' rather than 'auth').

I believe another feature that would improve admin index page usability a lot belongs conceptually here.

Suppose I have a project that contains 10 applications, each containing several models. Some of the apps are of primay importance, some are less important. Currently, there is no way to impose either ordering or hide app contents. Thus it's hard for users to discern important bits from non-important ones and confusion is guaranteed.

So I propose the following addition to this patch:

• ordering: app('mypkg.auth', 'myauth', 'My cool auth app', weight=1) # heavier items sink to the bottom in admin index page
• collapsing: app('mypkg.auth', 'myauth', 'My cool auth app', style={'classes' : ('collapse',)}) # similar behaviour to fieldsets in admin index page

See DjangoSpecifications/NfAdmin/FlexibleAppHandling