My votes for the "do we" issues are that we can leave gfk for internal usage. In some senses gfks really are a category of their own. For the RelatedObject issue I think we should accept the backwards incompatible change. In the long run I think we should aim for a situation where "RelatedObject" classes are just another implementation of the Fields API (they would be virtual fields automatically created on the reverse side of any relation). Changing the model attribute's content is a small step towards that goal.
- Anssi On Mon, Dec 22, 2014 at 9:39 AM, Russell Keith-Magee <[email protected]> wrote: > Hi all, > > The deadline for 1.8 alpha is rapidly approaching, and one of the features > that has been proposed for inclusion is Daniel Pyrathon's GSoC project > refactoring the _meta object. > > A huge thanks to Tim, Carl, Collin, Loïc, Anssi, and everyone else that has > provided detailed reviews and feedback - those review notes have been > invaluable. And a huge thanks, of course, to Daniel, who has done all the > heavy lifting of actually *writing* the patch. > > The purpose of this thread is to establish what is standing in the way of a > merge. The PR isn't 100% ready for merge - there are a couple of code > cleanups still required, and a handful of small design issues that still > exist. I'd like this thread to either resolve these design issues, > acknowledge that the issues can be deferred to post-Alpha, or get the > discussion to a point where the technical board has a concrete proposition > to resolve, or clearly establish that this feature isn't going to happen for > 1.8. > > The design issues I'm aware of aren't especially fundamental issues - > they're all mostly about naming. They're not quite bikesheds, though, > because the names are either significant, already in use, or likely to > change meaning in the near future. So - it's important that we get the names > right, but the important thing isn't really the names themselves - it's the > concept underpinning the names. > > The code and docs as it stands can be found on Github: > > https://github.com/django/django/pull/3114 > > In this discussion "Old" Meta refers to the older, officially non-supported > API in Django 1.7 and earlier. The "New" Meta is this patch - the API > developed during GSoC as a candidate for 1.8. > > Summary of API > ============ > > Documentation for the new API is available on the PR, but as a quick summary > - the proposal in this pull request is to formally document 2 methods on > _meta: > > * get_field(name) > > Returns a single field instance with the given name. > > get_field() exists in the old API, but it also accepts an 'many_to_many' > argument; this argument will be deprecated as part of the new API. > > * get_fields(include_parents=True, include_hidden=False) > > Returns a list of fields associated with a model. > > include_parents controls whether the list of fields is those defined > literally on this model, or if fields on the parent model are also included > (for inheritance situations). > > include_hidden controls whether hidden fields are returned. Hidden fields > are fields that exist for internal purposes, but aren't part of public API, > or have been explicitly hidden by a user. For example, the reverse relation > associated with a ForeignKey with a related_name that starts with "+" is a > hidden field - the reverse relation exists, but is hidden from public use. > > Proposed API usage > =============== > > The old API contained a large number of API endpoints (e.g., > get_concrete_fields_with_model()), each of which was tuned for a specific > use cases. Each of these API endpoints had their own caching scheme, and > there were some interesting inconsistencies and duplication in the return > values. > > The new API replaces this large number of methods with two primitives from > which all the other values can be derived. The intention is that instead of > having a specific method that returns exactly the right results for a single > use case, the new API will provide a generic list of fields, and provide the > tools to filter/extend results > > So - instead of providing "MyModel._meta.get_concrete_fields_with_model()", > users of the new _meta API will use: > > [(f, f.model) for f in MyModel._meta.get_fields() if f.concrete] > > For backwards compatibility, there is an implementation of all the old > methods using the new API. However, Django's own code no longer uses these > methods, and they have been marked as deprecated and will be removed in a > future release. > > This approach takes the focus off having a comprehensive API for finding > every possible field type, and moves the focus to having flags on each field > that enable end-users to identify the types of fields that are appropriate > for any given use case. > > The flags currently defined in the docs are: > > * concrete - the field has an underlying database column. > > * editable - the field is designed to be user-modified. e.g., AutoFields > are non-editable. > > * has_relation - the field represents a relationship with another model > > * hidden - fields that are concrete, but aren't intended to be used > directly - they're backing for some higher-level functionality (e.g., the > content_type and object_id columns for a GenericForeignKey) > > All fields also have a `model` attribute that describes the model that holds > the field. > > Fields that define has_relation=True will also define flags for the > cardinality of the relation they define: > > * one_to_one > * many_to_one > * one_to_many > * many_to_many > > All relation fields also have a `related_model` attribute that describes the > model that the relation is pointing to. If the relation is not to a single > type (e.g., GFKs), this value is None. > > In future, if we add a new field type, the intention will be to add new > flags as required to allow those new field types to be differentiated. This > won't affect most "data" columns (e.g., a hstore or JSON field) - it's for > fields that have unusual underlying representations, like Composite fields. > > Outstanding issues: > =============== > > This API works quite well for Django internally; as I indicated earlier, all > the existing Django code has been migrated to use the new API. However, > there are two outstanding design issues that I am aware of. (If there are > others that I've missed, please raise them!) > > * Handling of Generic Foreign Keys (esp in admin) > > In the old API, get_fields() didn't return GenericForeignKeys. The new API > does, but they can be filtered out. > > The only place this really matters in Django itself is in contrib.admin, > which can't display widgets for GFKs; as a result, they need to be manually > excluded from field lists. > > This is currently being done with an internal method named > `_get_non_gfk_field()`. > > In his review notes, Carl commented that we should be avoiding > field-specific names like "GFK", in favour of capturing the underlying > concept. This is a fair point, but leads to the question of what the > "underlying" concept is here. > > At one level, there's a reasonable argument that GFKs can be singled out > here - the use is an internal method, in contrib.admin, and GFKs are the > only field type admin doesn't support out of the box. > > At a higher level, though, a GFK is the only example of a virtual field in > Django - a field. This might change, however - some of the Composite Field > changes, plus possible change to ForeignKey might lead to more "virtual" > fields being introduced - some of which *will* be visible in the admin. > > The question: Do we - > > 1) Accept this particular internal specific naming of GFK as a quirk > reflecting the limitations of contrib.admin > > 2) Try to nail down what a "virtual" field means, or find some alternative > flag to identify what "GFK" is a proxy for in this case. > > In my opinion, (1) is acceptable at the moment; when Composite Keys (and > possibly the ForeignKey refactor) land, we can revisit what virtual means > and possibly address this issue in admin at the same time. It is for this > reason that I don't think virtual has a place in formal documentation for > 1.8, even though the flag exists, and is used by GFKs. > > * The "model", "parent_model" and "related_model" properties on fields. > > The new API ensures that every field has a `model` attribute, describing the > model that owns/defines the field. > > However, there's a backwards compatibility problem. RelatedObject - the > object used to represent the "other" side of FK and M2M relations - is also > returned by get_fields(), and *it* uses the '.model' attribute to store the > model that defines the field that the RelatedObject is managing. So - if > Book has a foreign key to Author, Author will have a RelatedObject named > book_set; and book_set.model will be Book. > > Related Objects also have a "parent_model" attribute that identifies the > other end of the relation - the model that is being pointed *at*. > > As part of this refactor, relation fields like ForeignKey have aquired a > "related_model" attribute that - so given a ForeignKey, you can tell what > model it is pointing at. > > This means there's an inconsistency here - if you think of RelatedObject as > being a "reverse" ForeignKey/M2M, then the model should be Author, and > related_model should be Book. The name "parent_model" doesn't make a whole > lot of sense in this context > > The question: Do we: > > 1) Accept this as a backwards incompatible change. > > 2) Accept the inconsistency in the new API. > > 3) Find a pair of names other than model/related_model to represent the > "subject/object" pair in fields > > My inclination is for (1). RelatedObject isn't stable API at present; the > whole purpose of formalising _meta is to formalise some of these historical > inconsistencies. > > RelatedObject is an internal implementation detail, and one that's only > visible if you're using the old "get_all_related_objects()" (and similar) > API. > > I think the inconvenience caused by making this change is worth it. The new > naming is internally consistent, and much less surprising; the set of people > who will be affected by this change is also the set of people who are > already spelunking through _meta; it's highly unlikely they're going to have > a completely painless 1.8 transition. > > Summary > ======= > > I think PR 3114 is pretty close to ready; I've highlighted my preferred > options for the two outstanding design issues, both of which are pretty easy > to resolve once a decision is made. > > Comments? Feedback? Objections? > > Yours > Russ Magee %-) > > -- > 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/CAJxq848T0EhY_foReSoxqDxcpNM-dy1K8CMqxTBZV6iysMYb5g%40mail.gmail.com. > 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/CALMtK1Gdk%2B%2BW_uWQszbc%2BsYiBjY8BbnAaULZN12gvGeC-f1dLQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
