On Sat, Aug 17, 2013 at 08:34:27AM +0700, Russell Keith-Magee wrote: > > 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.
Of course, however, as I wrote, as far as ModelForms and the admin are concerned, the extra fields have the `editable` flag set to False, which means they won't show up neither in the admin nor in any ModelForms by default. Of course, there may be people who examine the list of fields directly to create their own forms and also ignore the `editable` flag, but in that case I'm willing to argue they are doing it wrong. > > 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. That's right, there's a conceptual difference, but what I was talking about was that from the perspective of the ORM, they are both handled in the same way most of the time. Sure, it is possible to distinguish them. Originally, I was thinking about a flag on fields, such as `virtual`, but Anssi chose a different way, just setting the `column` attribute of virtual fields to None [1], which has been in the ORM for virtual ForeignObjects for some time now. (Although ForeignObject itself is not documented and people aren't supposed to be using it directly. There are tests for it, though.) Anyway, there are already properties such as `concrete_fields` and `local_concrete_fields` [2], it probably wouldn't hurt adding equivalent `virtual_fields` and `local_virtual_fields`, although I personally don't see much value in those. > 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. I'm certainly aware of that. Anyway, after spending several years working on this project, it is clear to me it isn't possible to do this without making *some* changes to _meta. So I went ahead and did such changes to make it as simple for me to get this project to a working state as possible, while at the same time I tried to simplify those things in _meta which I had to touch anyway. (I seem to have failed in this simplifying effort, though, because of GenericRelation and its need to be cloned. \-: ) That said, I'm not opposed to trying to keep things as close to their current state as possible. I only claim that it is nearly impossible to fully implement composite fields and at the same time keep things in _meta intact. > 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. I'll gladly take part in such an effort -- I do want to get this feature merged, after all. The biggest problem I see with this is that it will need other people to step in and get involved. Obviously, when I'm left to do things on my own, I make changes people are not appy with. But that's to be expected, I guess, I don't have that much experience to be able to strike the right balance between changing things and keeping them backwards compatible. However, while people usually show great interest in having this feature implemented, I haven't had much success getting people to help out with it. Too bad I'm not going to Djangocon, that would probably be a perfect chance to advance this in the right direction. Unfortunately, it looks like I missed the opportunity to get financial aid by one week. \-: Michal [1] https://groups.google.com/d/msg/django-developers/m8rO3Id5j-o/wL036hVhQO0J [2] https://github.com/django/django/blob/master/django/db/models/options.py#L281
signature.asc
Description: Digital signature
