Code

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#746 closed defect (invalid)

refactor models/__init__.py so fields themselves add their own get_XXXX_count/list functions

Reported by: Ian@… Owned by: rjwittams
Component: Core (Other) Version:
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

currently in models/__init__.py there is a big for loop which goes over all the installed modules
and creates the foreign key relationships (oneTomany,manytomany and one to one) with a big if/else/elif statement.

I propose we move the if/elif logic to the actual field code itself. so a field 'knows' what functions it has to create, and field writers could override this functionality to add their own relationships if they choose

from

for related in klass._meta.get_all_related_objects():
 if foo:
   ....
 elsif blah:
   ....

to

 for related in klass._meta.get_all_related_objects():
    related.create_relationships( klass, meta )

and

class OneToOne:
  def create_relationships( klass,meta )
    func = curry(....)
    ...
    klass.setattr(...)

I think this change would be backwards compatible. and could allow me to define a 'TagField' which creates special functions.

Attachments (2)

meta_models.patch (9.3 KB) - added by ian@… 8 years ago.
Meta Model Patch
meta_models_2.patch (9.7 KB) - added by ian@… 8 years ago.
now hangs of the fields instead

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by ian@…

Meta Model Patch

comment:1 Changed 8 years ago by ian@…

or is it better to actually have this function on the XXXField instead of XXX ?

Changed 8 years ago by ian@…

now hangs of the fields instead

comment:2 Changed 8 years ago by rjwittams

  • Keywords new-admin added

This patch is against the new-admin branch, its probably worth mentioning that.
In its current form it has no chance of working with trunk. Its almost certainly not worth backporting to trunk, either it should get applied in new-admin, or applied after new-admin merges.

This is something I had a comment about doing in new-admin so I'll have a look at it and see if it is backwards compatible.

comment:3 Changed 8 years ago by anonymous

Ok, it looks ok-ish so far. I'd prefer it if the scope was expanded to cover all field-provided methods, not just the relationship ones, eg date, choice, image,file methods. Passing "meta" as an argument seems odd as well, this is just a module, right? The functions to be curried and stuffed onto the class and module should be in fields.py, perhaps as staticmethods of the field classes.

So setup_relationships is probably a misnomer - I think we need something like "mutate_module" for adding module functions, and "mutate_class" for instance methods. You'll need to look at all the places the class and module are added to work out the appropriate arguments for these functions.

comment:4 Changed 8 years ago by ian@…

If you can point me to where they are defined now that would be great.

the meta is passed as an argument, as I couldn't figure out a way to reference it without passing it (I'm a python newbie, so it might be blantantly obvious)

I'm not sure why we need to seperate functions (one for class methods, and the other for instance methods).

maybe setup_field_functions?

regards
Ian
ps.. the code works as-is in my apps.

comment:5 Changed 8 years ago by rjwittams

So there are two things that a field might want to contribute methods to :

  • The magic model module
  • The generated model class

So I thought it would make sense to do this in two functions.
The functions are defined at the moment in django/core/meta/__init__.py

So the idea is it would just loop over all the fields, asking for their contribution to the class and the module (this might need to be done in two loops or something).
So maybe the methods should be called 'contribute_to_class', and 'contribute_to_module'.

I'm not sure about the ordering here, at the moment the relationship related methods are setup here, and everything else is setup in the metaclass. So I guess this needs a bit of thought.

So the meta : it looks like the only reason you need this is to access the curried functions - these should probably become static methods of the field or something. Other than that, just import django.core.meta, or the individual functions from there.

I might have a better look into this tomorrow.

comment:6 Changed 8 years ago by rjwittams

  • Keywords new-admin removed
  • Owner changed from adrian to rjwittams

Ok, so this is included in my mutually referential models svk branch. I will make a ticket for that soon and upload a patch. If not, bug me.

comment:7 Changed 8 years ago by adrian

  • Resolution set to invalid
  • Status changed from new to closed

Closing this, as it's out of date for magic-removal.

comment:8 Changed 8 years ago by Link

  • Type changed from enhancement to defect

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.