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]> 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]. > 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/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]. 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%2Bm8oA8upah5K9N2FOnT6fDaPiWwfNi0ykYTSvCX_0MYmfRjmg%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
