Hi Thomas, I replied earlier before you posted, looks like my message got sent actually a lot later.
having a function like *models.constrain(x, y, unique=True) *makes sense for inline declarations. About null handling, If all columns on the database are null=True and all columns have a null value, then the composite field value would be `None'. Otherwise it would be a dict mapping to the subfields, and a query needs to specify the subfields it queries against. Aron. On Saturday, March 7, 2015 at 11:16:12 PM UTC-5, Thomas Stephenson wrote: > > Aymeric, > > Thanks for your input. I feel some of your concerns have been addressed in > the DEPs I made, which have included quite a bit of input from this thread > along with the original design. That said, some of the points you've raised > are new and haven't been raised by other people, so I'll give you a full > reply. > > 1) That's still something I have to do, but more time consuming than other > parts of creating the proposal and I've put it off until I have a while to > do it. The original implementation was done without too much consideration > of prior attempts, because I was more interested in getting something > working than I was in learning from the past. > > 2) I agree. The new syntax I've proposed is to add one or more class-level > methods to the django.db.models API. The new syntax for "inline" fields is: > > class MyModel(models.Model): > x = models.IntegerField() > y = models.IntegerField() > point = models.constrain(x, y, unique=True) > good > > The reason for not using the CompositeField constructor is that a lot of > the options which make sense for standalone field constructors (and all the > other field classes) make absolutely no sense when you're mainly leveraging > composite fields to provide a table level constraint to two existing > fields. Also there are some things that are commonly done in practice (like > providing `verbose_name` as a positional arg) that make adding multiple > leading positional arguments difficult. > > Note: It's a shame that we can't use py3's keyword only arguments here. > > 3) No, not all the field base API makes sense for composite fields -- in > fact most of it doesn't. This is a huge problem with fields in general, not > just composite fields -- there's just too much functionality on the basic > Field class that assumes a one-to-one mapping between a Field and a > database column and too many parameters accepted by field base that only > make sense for subsets of the available field types. > > e.g. > - What does "blank" or "max_length" mean for an IntegerField? > - What does 'rel' mean for a NullBooleanField? > - What does get_col mean for a ManyToManyField? > etc. > > To fix this would mean introducing significant backwards incompatible > changes and, while I would support such a change, I'm not sure it's a > discussion that affects the ability to implement composite fields. > > 4) The addition of the `isnull` field is mainly to support the ability to > query for whether a composite field is NULL. If we leave it implementation > defined as to how to interpret whether a specific configuration of > subfields maps to a python `None` value for the composite field, then it > becomes really difficult to define lookup transformations to query for null > values in the table. > > But although it sounded like a good idea when I came up with it, it raises > more questions than it solves. I'm more than ready to go back to the > drawing board on that one. > 5) Inheritance in the model API is complicated enough as it is without > adding inheritance of field types. Yes, it could be supported by "beating > the metaclass into submission", but it's not something that I think adds > any particular value. I did change the "no subclassing at all" restriction > to "only one class in an inheritance heirarchy can define subfields" when > writing the DEP though, because Ansarri wants the ability for users to > extend ForeignKey with their own python behaviour. > > Thomas > > > > On 7 March 2015 at 22:31, Aymeric Augustin <[email protected] > <javascript:>> wrote: > >> Hello Thomas, >> >> It’s hard for me to digest a two-page-long email on a complex topic during >> the week. I bookmarked your first email when it came in. It’s Saturday, >> 11am, >> and I dedicated my first chunk of quality brain time to reading the entire >> thread. I’ll let you ponder what the effect of your second email was. >> >> In fact, if you propose something stupid, you’ll get a quick answer, >> because >> it’s easy to explain. Not getting answers right now means that your >> proposal >> is good enough to require consideration. Yes, that’s counter-intuitive. >> >> With that out of the way, here are my thoughts on your proposal. Some >> overlap >> with what other people have said in the thread. >> >> 1) I would find it reassuring if you described the landscape of past >> attempts, >> what good ideas you’re keeping, what bad ideas you’re throwing away — in >> short, if you convinced us that you’re building on top of past attempts. >> The >> main goal of this exercise is to guarantee that you don’t overlook a use >> case >> or an argument that made consensus in the past. (Don’t spend too much >> time on >> code; it my experience it’s harder to reuse and code matters much less >> than >> design. >> >> 2) The syntax for inline composite fields doesn't look very nice. Could >> you >> simplify it somehow? Anssi’s proposal is good. I assume that a composite >> field >> could add the subfields to the model class if they aren't defined >> explicitly >> and their names passed in arguments to the composite field. >> >> 3) Have you checked that all the Field APIs make sense for CompositeField? >> It's quite obvious that value_from/to_dict are needed. It's less obvious >> that >> everything else is still needed. >> >> 4) I'm wary of the extra 'isnull' column. Couldn't we required that, if >> the >> composite field is NULL, at least one of the subfields is NULL? My idea is >> that a nullable composite field would consider itself NULL if all nullable >> subfields are NULL. Declaring a composite subfield nullable when all >> subfields >> are non-nullable would be an error. In your MoneyField example, the amount >> subfield should be nullable when the composite field is nullable. >> >> 5) I understand the first two restrictions. They required deeper >> refactorings >> to be lifted. The reason for the third one is less clear. Is it just a >> matter >> of beating the metaclass into cooperation? >> >> Best, >> >> -- >> Aymeric. >> >> -- >> 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:>. >> To post to this group, send email to [email protected] >> <javascript:>. >> 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/2130B21B-41FA-4928-BC9C-5F5BAADAD7E0%40polytechnique.org >> . >> 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/aff3be14-2f93-4d06-b69e-7fa1877247a4%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
