Django

Code

Ticket #2982 (closed: duplicate)

Opened 2 years ago

Last modified 10 months ago

[patch] allow replacing models.py with a subpackage models/__init__.py

Reported by: Bastian Kleineidam <calvin@debian.org> Assigned to: nobody
Milestone: Component: Database wrapper
Version: Keywords:
Cc: ferringb@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 1

Description

Hi,

if you have a lot of models, or even a model hierarchy, one might want to convert the plain models.py into a package models/__init__.py. However, the ModelBase class cannot cope with models in subpackages, since it always chooses the second-last module part as app name. This patch fixes this by looking for the module part before models.

Attachments

django_models.diff (1.1 kB) - added by Bastian Kleineidam <calvin@debian.org> on 11/06/06 13:13:04.
model-api.diff (1.0 kB) - added by Marc Fargas <telenieko@telenieko.com> on 01/18/07 16:04:55.
Documentation for #2982

Change History

11/06/06 13:13:04 changed by Bastian Kleineidam <calvin@debian.org>

  • attachment django_models.diff added.

11/06/06 13:21:02 changed by anonymous

  • summary changed from allow replacing models.py with a subpackage models/__init__.py to [patch] allow replacing models.py with a subpackage models/__init__.py.

11/06/06 14:37:08 changed by ubernostrum

I don't know offhand if it's documented anywhere (if it isn't we need to fix that), but splitting models up into files in a module actually works right now -- you just have to set the Meta option app_label to the name of the app, and make sure the __all__ statement in the module's __init__.py exports all the files you want to use.

11/06/06 21:31:26 changed by Gary Wilson <gary.wilson@gmail.com>

There is some documentation at CookBookSplitModelsToFiles.

There used to be a small section in the old model_api docs, but that seems to have disappeared.

11/07/06 07:08:15 changed by Bastian Kleineidam <calvin@debian.org>

I did not see the Meta.app_name setting. With the patch you don't need to set the Meta.app_name anymore. Just move the Models in a subpackage and be done.

And it would be nice to have the models-as-subpackage doc paragraph back, I surely missed that :)

01/18/07 01:00:54 changed by SmileyChris

  • needs_docs set to 1.
  • stage changed from Unreviewed to Ready for checkin.

Looking good, Bastian.

Some documentation could be good, but I don't think it blocks this being checked in.

01/18/07 16:03:48 changed by Marc Fargas <telenieko@telenieko.com>

  • stage changed from Ready for checkin to Accepted.

Stripping the "Ready for checking" as I'm about to upload documentation for that, please review it and set "Ready for checking" if it's ok.

Hope you don't mind ;)

01/18/07 16:04:55 changed by Marc Fargas <telenieko@telenieko.com>

  • attachment model-api.diff added.

Documentation for #2982

01/19/07 03:05:17 changed by SmileyChris

  • needs_docs deleted.
  • stage changed from Accepted to Ready for checkin.

Apart from a few weird character typos in the documentation, it looks all right to me. I'm sure a core will check the wording / typos and change as appropriate.

02/10/07 00:43:47 changed by anonymous

  • needs_better_patch set to 1.
  • needs_tests set to 1.
  • stage changed from Ready for checkin to Accepted.

Not quite ready for checkin yet, upon examination (no fault of the triagers, I just was looking over it before checking it in)...

In the past we've rejected this sort of approach because it required the redundant import statements in models/__init__.py (rather than just from models.foo.bar import MyModel in the code being sufficient). Ideally, it would be nice to descend from models (whether it's models/ or models.py) and extract all the models without needing the models/__init__.py import. I think db.models.loading is one place that needs fixing for that. I think it should be possible to make this work.

We should be able to make a test for this one, since we have models.py files in the tests already (make a new test directory and have a models/ directory in it). Need to make sure we test that it works sufficiently generally (models/foo/bar.py should work -- looks like it does, but testing makes perfect).

I don't like the use of "assert" in this patch, since assert is for things that should never fail and we can "guarantee" will be true. We cannot make guarantees about the user's (programmer's) code, though; they do make mistake and it's not Django's internal fault if that happens. So we should do a normal check and raise a proper exception (not AssertionError?) if there is a problem. Everywhere else in Django does this upon an end-user error, so it's consistent.

I'll probably fix some/all of these problems myself on the next pass through if nobody wants to do it first. Just recording the problems for future triage benefit.

06/03/07 05:24:39 changed by Brian Harring <ferringb@gmail.com>

  • cc set to ferringb@gmail.com.

09/16/07 10:02:03 changed by ubernostrum

  • status changed from new to closed.
  • resolution set to duplicate.

This is yet another issue related to app_label, and so gets closed in favor of #3591.


Add/Change #2982 ([patch] allow replacing models.py with a subpackage models/__init__.py)




Change Properties
Action