Opened 16 years ago

Closed 16 years ago

#7852 closed (invalid)

Cross-application model inheritence broken after newforms-admin merge

Reported by: jbalonso Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords: model inheritance
Cc: jalonso@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have an application that needs to register models for the admin interface in one app that also has a base class for models in two other apps. After the newforms-admin merge, it is impossible to import the base class into another app, since it needs to be referenced by an app.model name string instead of the class reference itself.

My recommendation is to create a models.base_class() function that accepts an app.model string and returns a dynamic class that can be derived. If such a beast already exists, it needs to be documented.

To clarify...

Old:

from django.db import models
from remote.models import Foo

class Bar(Foo):
    # etc.

Proposed:

from django.db import models

class Bar(models.base_class('remote.Bar')):
    # etc.

Change History (4)

comment:1 by jbalonso, 16 years ago

Cc: jalonso@… added

comment:2 by Malcolm Tredinnick, 16 years ago

That proposed API is terrible. We are programming Python here and subclassing has a well-established syntax. It looks like something about the new admin code's importing needs to be fixed. Certainly "breaking" Python isn't the solution here.

in reply to:  2 comment:3 by jbalonso, 16 years ago

Replying to mtredinnick:

That proposed API is terrible. We are programming Python here and subclassing has a well-established syntax. It looks like something about the new admin code's importing needs to be fixed. Certainly "breaking" Python isn't the solution here.

Very well. Then I suggest the API for registering models for the admin interface either be taken out of models.py files entirely or wrapped in a class so the registration code isn't run on every import.

I think any solution to this that will require a backwards-incompatible API change. As such, I think this should be resolved by the 1.0 release. Being a relatively rookie ticket submitter, I'm not going to set the milestone myself, though.

comment:4 by Brian Rosner, 16 years ago

Resolution: invalid
Status: newclosed

There already is a convention for registering models with the admin interface that does not involve models.py. Originally models.py was the only decently safe route before an admin.py module was introduced for applications. All ModelAdmin classes should be placed in an admin.py file in the app. Registration with the default AdminSite instance should be done there only. If you have a need for your own AdminSite then do so at a project-level admin.py and do registration there. The admin.autodiscover() function will take care of doing the imports on the admin.py modules of the INSTALLED_APPS. See the documentation for more information. I am assuming you are seeing AlreadyRegistered exceptions, if so keep this closed, otherwise re-open with specific details.

Note: See TracTickets for help on using tickets.
Back to Top