On Thu, Aug 15, 2013 at 6:56 PM, Michal Petrucha <[email protected]>wrote:
> Hello, > > As part of my GSoC work [1] I did a refactor of ForeignKey whereby I > turned it into a virtual field which creates an auxiliary concrete > field to hold the raw value. > > As part of this, there are several changes that I did in Model._meta: > > 1) There are now more fields in _meta.fields, obviously. For each > ForeignKey there is now an extra field with `_id` appended to its > name. This extra field is not editable nor serializable, which > means things like serialization, ModelForms or the admin just > ignore it, however, I have no idea what other things people use > _meta.fields for and it might cause problems for third-party > projects. > I can imagine this causing *massive* problems -- for example, in the admin (or just on form sets, for that matter), the "list of fields" is used to dynamically generate a list of elements needed on a form. If every ForeignKey is now on the list twice, there's going to be a *huge* set of changes required. > 2) I ditched _meta.virtual_fields and put all fields into local_fields > instead. The reason is simple, since ForeignKey is now virtual, in > a lot of places where local_fields is accessed, I'd have to search > local_fields + virtual_fields. Basically, the only reason for > virtual_fields that I could find was that they were cloned in each > model subclass, more on this in the next item. > Well, the reason is that the virtual fields aren't *exactly* like normal fields -- they don't literally exist on the database, for example. So, if we're merging virtual_fields with local_fields, there needs to be a way to tell the two apart. > 3) No more cloning of all virtual fields in each model subclass. > Currently, each field in virtual_fields in parent models is cloned > in ModelBase.__new__ and added to the subclass as a separately > owned field. This is rather redundant for most virtual field types > (in master that would only be GenericForeignKey and > GenericRelation, in my composite-fields branch there's also > ForeignKey and CompositeField). The only "field" which requires > cloning is GenericRelation which needs to determine when it's > accessed through a model subclass to retrieve the right content > type. > > For other field types this is not necessary and in the case of > ForeignKey it might even cause undesired effects, like multiple > conflicting reverse ForeignKey pointers to a target model. Also, > all related object descriptors and possibly other objects would be > recreated separately for each subclass. While those should be > possible to deal with, I'm still not a big fan of creating clones > of objects and adding extra complexity to account for this without > any real reason. > > Therefore, instead of cloning all virtual fields, each field class > now has an attribute `clone_in_subclasses` which is True in > GenericRelation and False everywhere else. Those fields with this > attribute set to True will end up in private_fields instead of > local_fields. > > To see the whole diff, check out the pull request I created [2]. > > I'd like people's opinions on these changes. What's the status of > Model._meta? Last time I saw, it was in the state of a semi-private > API -- it is not really public as it is not documented anywhere, but a > LOT of people still use it in their projects because it's considered > quite stable. > > As I said, I don't really know what people use _meta for in their > projects and it's rather difficult for me to anticipate in what ways > these changes can break things for them. > > Anssi suggested in a comment [3] to keep fields as close to the > current state as possible, which means ForeignKey would appear there > and the auxiliary fields it creates would not and create something > like all_fields which would hold those as well. It is certainly > possible to do this, but personally, I'm not really convinced -- it > would mean I'd have to rewrite practically all occurrences od > _meta.fields in Django to _meta.all_fields and it would introduce > certain ambiguity as it would not really be clear what the difference > between fields and all_fields is. > > So, I'd like your opinions. The more the better. Whatever the outcome, > all changes will be thoroughly documented in the release notes which > I'll start working on as soon as a conclusion is reached. > Ok - so my opinion is that _meta needs to be treated as "unofficially stable" API. If you're proposing changes to _meta, we need to be *very* careful about those changes, because there are lots of people that are relying on the current internal structure of _meta. And while it isn't *officially* stable API, we won't win any friends if we start massively tinkering with it. There's definitely a *lot* of improvements that can be made to the internals of _meta. It's a bit of a mess, with all sorts of things being cached in weird ways, and being retrieved in odd patterns. However, I'd rather see that done as a consolidated effort, rather than as a set of little tweaks to make this new feature work -- especially if those little tweaks are going to massively change the status quo. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
