I think we can separate composite field implementation into two steps, first implement composite fields for user defined existing fields, then make it possible for fields to auto-create its subfields.
I'm not sure of the meta issue. It seems in some cases you'll want access to the subfields, especially if foreign keys are going to be split using this same idea. +1 for DEP, this is exactly the kind of issue where writing a DEP will be usefull, and getting an acceptance for the proposal will make implementation easier. - Anssi On Friday, March 6, 2015, Thomas Stephenson <[email protected]> wrote: > I'll reiterate that I mispoke when I said that "subfields wouldn't be > added to model._meta". They would be, it's just that the contributor would > be the CompositeField, not the model. Subfields are just wrappers for other > field types and need a reference to the composite field. The difference > that I was trying to make when I said "would not be available on > model._meta" is that they wouldn't be available via the public API without > passing special flags. > > I'm all for creating a DEP for the proposal, it seems like a better place > for nutting out these issues than a mailing list thread. What's the current > index into the deps? I'm guessing that it's 191. > > Thomas > > On 6 March 2015 at 03:13, Markus Holtermann <[email protected] > <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: > >> While I like your approach, Thomas, I should also note that I'm not an >> expert on those parts of Django and haven't kept up with the Michal's >> proposal. >> >> With respect to migrations I like Anssi's approach to have both `author` >> and `author_id` in _meta.fields. It will probably simplify a couple of >> things and we can get rid of some workarounds. >> >> Furthermore, I suggest that we take the time and proceed with a DEP ( >> https://github.com/django/deps) to find common ground what the API >> should look like before we rush a concrete implementation. >> >> /Markus >> >> >> On Thursday, March 5, 2015 at 4:07:09 PM UTC+1, Thomas Stephenson wrote: >>> >>> Well, I was thinking that although the subfields would still be added to >>> Model._meta.fields, you could >>> >>> - add an 'include_subfields=False' default argument to get_fields() >>> - only allow direct lookups of subfields via Model._meta.get_field( >>> composite).get_subfield(subfield) or (alternately, depending on >>> people's tastes) Model._meta.get_field(composite__subfield) >>> >>> Thomas >>> >>> On Friday, March 6, 2015 at 1:58:26 AM UTC+11, Marc Tamlyn wrote: >>>> >>>> I think `Model._meta` may well always want all the concrete underlying >>>> fields included somehow, especially for migrations work. A possibility for >>>> your external `MoneyField` could be to add other fields using >>>> `contribute_to_class`, though this may be a bad idea... >>>> >>>> On 5 March 2015 at 14:47, Thomas Stephenson <[email protected]> wrote: >>>> >>>>> > Turns out this was a bad idea, after around 1700 lines changed >>>>> everything was broken and there were multiple hard to debug failures. >>>>> >>>>> Yeah, been in this situation too many times to count. >>>>> >>>>> > Split ForeignKey to a concrete field instance, and a related field >>>>> instance. After this all related fields are purely virtual. This means >>>>> that >>>>> author = models.ForeignKey(Author) will automatically generate a author_id >>>>> = IntegerField() on the model. Unfortunately this also means model._meta >>>>> will now contains two fields for each foreign key defined on the model >>>>> instead of just one. >>>>> >>>>> The foreign_key_id field could be a subfield of foreign key (then >>>>> model._meta would not contain two entries for the foreign key). >>>>> Unfortunately that would open the door to composite field inheritance, but >>>>> it could be handled like enum inheritance is handled in python -- you can >>>>> subclass as much as you want, but you can't declare additional fields on >>>>> subclasses. >>>>> >>>>> If that happened, then composite foreign keys would be a lot easier >>>>> (but still work that can be deferred). >>>>> >>>>> > Michal Petrucha did a lot of work to add composite fields to >>>>> Django. The syntax he had was: >>>>> >>>>> I'm not exactly a fan of that syntax. It works for the unique_together >>>>> and index_together use cases (and the primary key use case), but it puts >>>>> all the subfields on model._meta and doesn't allow you encapsulate the >>>>> behaviour of composite fields outside the model definition. So you can't, >>>>> for example, define a reusable `MoneyField` that represents two columns in >>>>> the target model. >>>>> >>>>> >>>>> > The first part in the composite fields work should be making the >>>>> point field example to work. This will mean supporting >>>>> .filter(point__in=((1, 2), (2, 3))), and support for .values('point'). >>>>> Both >>>>> of these will be surprisingly complex to do correctly. In addition there >>>>> will likely be a lot of work to do in other parts of Django, too (for >>>>> example in migrations), so implementing just "simple" composite fields >>>>> will >>>>> be a lot of work. >>>>> >>>>> Well, I've already got that working (well, I've got point__exact >>>>> working and I can add point__in easily enough, it's just a matter of >>>>> adding >>>>> the relevant lookup transformations to get_lookup_transform. There were >>>>> some comments surrounding that function which suggest it needs a >>>>> refactoring, but I don't think it does. >>>>> >>>>> Thomas >>>>> >>>>> On 5 March 2015 at 22:30, Anssi Kääriäinen <[email protected]> wrote: >>>>> >>>>>> I've started doing some refactorings to the fields/related.py. The >>>>>> ultimate goal is to have field.rel (ForeignObjectRel) instances to be >>>>>> Field >>>>>> subclasses. >>>>>> >>>>>> I first went ahead and did exactly this with the idea of changing >>>>>> everything in one go. Turns out this was a bad idea, after around 1700 >>>>>> lines changed everything was broken and there were multiple hard to debug >>>>>> failures. >>>>>> >>>>>> I did a fresh start, and my plan is now to do the following: >>>>>> - First, make ForeignObjectRel to act like a Field instance (part >>>>>> of this is done in https://github.com/django/django/pull/4241) >>>>>> - Make ForeignObjectRel Field subclass. This will likely rename the >>>>>> classes to something like ReverseForeignKey, ReverseManyToMany and so on. >>>>>> - Finally, add the new reverse field instances directly to the >>>>>> remote model's _meta >>>>>> >>>>>> This is just clean-up in the fields/related.py. The composite fields >>>>>> work doesn't need to rely on this. To get to a state where we have >>>>>> composite primary keys and composite joins we should: >>>>>> - Split ForeignKey to a concrete field instance, and a related >>>>>> field instance. After this all related fields are purely virtual. This >>>>>> means that author = models.ForeignKey(Author) will automatically >>>>>> generate a >>>>>> author_id = IntegerField() on the model. Unfortunately this also means >>>>>> model._meta will now contains two fields for each foreign key defined on >>>>>> the model instead of just one. >>>>>> - Add composite fields (but not yet composite foreign keys or >>>>>> primary keys) >>>>>> - Add composite primary keys >>>>>> - Add composite foreign keys >>>>>> >>>>>> Addition of composite fields can be done at the same time with >>>>>> changes to fields/related.py, so it should be possible to start working >>>>>> on >>>>>> composite fields right away. >>>>>> >>>>>> Michal Petrucha did a lot of work to add composite fields to Django. >>>>>> The syntax he had was: >>>>>> >>>>>> class MyModel(models.Model): >>>>>> x = models.IntegerField() >>>>>> y = models.IntegerField() >>>>>> point = models.CompositeField(x, y) >>>>>> >>>>>> I think we should stick to that. >>>>>> >>>>>> It is essential that we don't try to do too much in one go. Even >>>>>> small changes tend to be hard to do. Trying to do all this in one go will >>>>>> result in a patch that will be nearly impossible to review, and which >>>>>> will >>>>>> conflict badly with other changes once finished. >>>>>> >>>>>> If you want to work on the composite fields part of this, I suggest >>>>>> you should take a close look at what was done in Petrucha's GSoC work ( >>>>>> https://github.com/koniiiik/django/, unfortunately I don't recall >>>>>> which branch contains the latest code). The code got to a point where it >>>>>> was pretty much commit ready. The commit didn't happen because migrations >>>>>> were getting ready for merge at the same time, and committing Petrucha's >>>>>> work would have caused severe problems for migrations. >>>>>> >>>>>> The first part in the composite fields work should be making the >>>>>> point field example to work. This will mean supporting >>>>>> .filter(point__in=((1, 2), (2, 3))), and support for .values('point'). >>>>>> Both >>>>>> of these will be surprisingly complex to do correctly. In addition there >>>>>> will likely be a lot of work to do in other parts of Django, too (for >>>>>> example in migrations), so implementing just "simple" composite fields >>>>>> will >>>>>> be a lot of work. >>>>>> >>>>>> I like the MoneyField idea, but lets punt composite field definitions >>>>>> in that way for later. >>>>>> >>>>>> - Anssi >>>>>> >>>>>> -- >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "Django developers (Contributions to Django itself)" 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. >>>>>> To view this discussion on the web visit https://groups.google.com/d/ >>>>>> msgid/django-developers/673dc9ed-da09-46d5-902e- >>>>>> 311756613c81%40googlegroups.com >>>>>> <https://groups.google.com/d/msgid/django-developers/673dc9ed-da09-46d5-902e-311756613c81%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>>> . >>>>>> >>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>> >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Django developers (Contributions to Django itself)" 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. >>>>> To view this discussion on the web visit https://groups.google.com/d/ >>>>> msgid/django-developers/CA%2Bm8oA8JDnOWHh%2BHXRgANjVLu31% >>>>> 3D34WC%2Btc1%2Bdu8QyQpySiAxA%40mail.gmail.com >>>>> <https://groups.google.com/d/msgid/django-developers/CA%2Bm8oA8JDnOWHh%2BHXRgANjVLu31%3D34WC%2Btc1%2Bdu8QyQpySiAxA%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> >>>> -- >> You received this message because you are subscribed to the Google Groups >> "Django developers (Contributions to Django itself)" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] >> <javascript:_e(%7B%7D,'cvml','django-developers%[email protected]');> >> . >> To post to this group, send email to [email protected] >> <javascript:_e(%7B%7D,'cvml','[email protected]');>. >> Visit this group at http://groups.google.com/group/django-developers. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/django-developers/3b3712f9-de74-424e-8e21-2af16003f8bd%40googlegroups.com >> <https://groups.google.com/d/msgid/django-developers/3b3712f9-de74-424e-8e21-2af16003f8bd%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> > > -- > You received this message because you are subscribed to the Google Groups > "Django developers (Contributions to Django itself)" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected] > <javascript:_e(%7B%7D,'cvml','django-developers%[email protected]');> > . > To post to this group, send email to [email protected] > <javascript:_e(%7B%7D,'cvml','[email protected]');>. > Visit this group at http://groups.google.com/group/django-developers. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/CA%2Bm8oA8upah5K9N2FOnT6fDaPiWwfNi0ykYTSvCX_0MYmfRjmg%40mail.gmail.com > <https://groups.google.com/d/msgid/django-developers/CA%2Bm8oA8upah5K9N2FOnT6fDaPiWwfNi0ykYTSvCX_0MYmfRjmg%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" 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. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CALMtK1G8qFMOt8GXURYc25x%2BJy4m6R1Vkc_VsY943_c0XQQccA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
