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