Delivering PR 3114 - The _meta refactor

[Russell Keith-Magee]

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 %-)

[Anssi Kääriäinen]

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

> --
> 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 django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> 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.

[Aymeric Augustin]

Hi Russell,

2014-12-22 8:39 GMT+01:00 Russell Keith-Magee <rus...@keith-magee.com>:

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.

I'm in favour of (1) mostly because of the YAGNI principle. Defining "virtual"  without a concrete use case sounds hard.

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

I'm also in favour of (1) because the point of a refactoring is to clean up inconsistencies. We can say something in the release notes to help people who have used the RelatedObject private API.

Best,

--

Aymeric.

[Collin Anderson]

Ditto. I'm ok with a tiny shim for GFK, and we should get the API right for RelatedObject going forward, (as long as it's at least _possible_ to detect which is which and write code that works on both 1.7 and 1.8).

[Andrew Ingram]

Hi all,

Just to add a data point regarding virtual fields.

We're using them in production in our open source media library (https://github.com/onefinestay/django-mediacat/blob/master/mediacat/fields.py#L97-L119). Essentially, due to a number of issues, it wasn't practical to have references to image crops to be foreign keys. Instead we have an association table with object_content_type, object_id, crop_id and field_name. The virtual field allows us to have an API that looks very much like a foreign key, but behind-the-scenes does all the magic to give us the benefits we were looking for. In the previous link, you'll also be able to see the hack I used to get the custom field working properly in modelforms.

We're also using them a lot in other projects to seamless bridging between database-backed models and other types, ie whenever we want something that looks like a database column as far as the model is concerned, but isn't one in reality.

Regards,

Andrew Ingram

--

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 django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
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/13baaf10-2395-47f6-8d1a-3a3b1543b7ff%40googlegroups.com.

[Markus Holtermann]

Hey folks,

On Monday, December 22, 2014 3:29:15 PM UTC+1, Aymeric Augustin wrote:

Hi Russell,

2014-12-22 8:39 GMT+01:00 Russell Keith-Magee <rus...@keith-magee.com>:

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.

I'm in favour of (1) mostly because of the YAGNI principle. Defining "virtual"  without a concrete use case sounds hard.

Exactly: (1) seems to be the better option

 

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

I'm also in favour of (1) because the point of a refactoring is to clean up inconsistencies. We can say something in the release notes to help people who have used the RelatedObject private API.

+1. Cleaning up the current "mess" and accepting backwards incompatibilities of an undocumented internal API.

/Markus 

[Carl Meyer]

Hi Russell,

Many thanks to Daniel, and to you, for all the work on this PR!

On 12/22/2014 12:39 AM, Russell Keith-Magee wrote:
> * 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.

One thing I've mentioned on the PR is that currently `get_fields` also
accepts `**kwargs`, which is used for some internal-only flags that are
used only when it recursively calls itself. I would prefer to see this
cleaned up so the API of the public method is precisely what is
documented, and no more (accepting `**kwargs` means that an end-user
typo of an argument name could result in silent wrong behavior rather
than an argument error), and if internal-only recursive behavior is
needed, use a separate internal method (called by the public method) for
that.

The issue that concerned me was not the name `_get_non_gfk_field` in the
admin, but the presence of a non-standard `is_gfk` flag on
GenericForeignKey, which was special-cased in several places, in
`_get_non_gfk_field` but also two places in the core ORM.

The `is_gfk` flag has since been removed and replaced by appropriate
capability flag checks on the field (in
https://github.com/PirosB3/django/commit/3f668766f67669bc9a19ae78e888840609e443c5).
So my concern has already been addressed. I don't have a problem with an
internal utility function in the admin named `_get_non_gfk_field`.

It's helpful to know what the current thinking on `virtual` is. It
initially seemed odd to me that this PR, whose goal was to clean up and
standardize field categories/flags, didn't define or make any use of
`virtual`, but also didn't remove it. From what you've said here, it
seems the thinking is "it may be useful in future but we don't have
enough examples yet to know how it should be used, so we'll keep it
around but not make it public yet." That seems OK to me. It might be
nice if that thinking were recorded in a code comment or docstring
somewhere.

I fully agree: if the idea is to clean up, let's clean up!

To be clear, does that mean getting rid of the `parent_model` attribute
entirely, in favor of a standardized `model` and `related_model`? That's
what I would favor.

The current code in the PR actually expands the use of `parent_model` so
that it is always set on all fields (and in the case of non-related
fields, is the same as `model`). I think this is not a good idea, for
two reasons:

1) As you noted, `parent_model` is a confusing name in this context.
`related_model` is a much better name.

2) For a non-has-relation field, it doesn't make sense for
`parent/related_model` to be set to the same as `model` - it would make
more sense for it to be `None`.

> 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.

A few other small issues that I would still like to see resolved:

1) Currently FieldFlagsMixin has a number of dynamic properties that are
only dynamic because they special-case checks for `is_reverse_object` --
I think it would be better if whatever sets `is_reverse_object` (only
`RelatedObject` I think?) simply switched the appropriate flags itself
(replacing a series of conditionals with polymorphic behavior).

2) Currently `FieldFlagsMixin` does not only contain flags, it also
contains a definition of the `related_model` property. Perhaps it should
be renamed to simply `FieldMixin` if it is not limited to flags?

3) I think it would be better if all the cardinality flags were present
on the base `FieldFlagsMixin` and defaulted to `None`. That would permit
many checks for e.g. `if field.has_relation and field.one_to_many` to be
safely replaced with the more concise `if field.one_to_many`. In general
I think `None` is a better choice than `raise AttributeError` for
standard flags that don't apply to a particular field.

4) There is an open PR (originally authored by Anssi, recently updated
by Tim) to remove RelatedObject entirely:
https://github.com/django/django/pull/3757 -- I think that's a good PR
and should be merged for 1.8, but it will require some non-trivial
updates to the _meta PR once it is merged.

Carl

Carl

[Russell Keith-Magee]

On Tue, Dec 23, 2014 at 4:18 AM, Carl Meyer <ca...@oddbird.net> wrote:

Hi Russell,

Many thanks to Daniel, and to you, for all the work on this PR!

On 12/22/2014 12:39 AM, Russell Keith-Magee wrote:
>  * 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.

One thing I've mentioned on the PR is that currently `get_fields` also
accepts `**kwargs`, which is used for some internal-only flags that are
used only when it recursively calls itself. I would prefer to see this
cleaned up so the API of the public method is precisely what is
documented, and no more (accepting `**kwargs` means that an end-user
typo of an argument name could result in silent wrong behavior rather
than an argument error), and if internal-only recursive behavior is
needed, use a separate internal method (called by the public method) for
that.

Sure - makes sense to me. At one point, I think we had the **kwargs there for future expansion; however, given the new shape of the API, I agree this isn't required.

Ah - that explains my confusion. Looks like Daniel removed is_gfk while I was in the middle of a review :-) 

Consensus seems to be that the admin-only GFK name isn't an issue, so I'll call this one resolved.

It's helpful to know what the current thinking on `virtual` is. It
initially seemed odd to me that this PR, whose goal was to clean up and
standardize field categories/flags, didn't define or make any use of
`virtual`, but also didn't remove it. From what you've said here, it
seems the thinking is "it may be useful in future but we don't have
enough examples yet to know how it should be used, so we'll keep it
around but not make it public yet." That seems OK to me. It might be
nice if that thinking were recorded in a code comment or docstring
somewhere.

Makes sense. I'll add that to the todo list.

Yes - I believe we should remove "parent_model". The only reason I can think to keep it around would be for backwards compatibility.

The current code in the PR actually expands the use of `parent_model` so
that it is always set on all fields (and in the case of non-related
fields, is the same as `model`). I think this is not a good idea, for
two reasons:

1) As you noted, `parent_model` is a confusing name in this context.
`related_model` is a much better name.

2) For a non-has-relation field, it doesn't make sense for
`parent/related_model` to be set to the same as `model` - it would make
more sense for it to be `None`.

 

Agreed on both counts. 

 

> 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.

A few other small issues that I would still like to see resolved:

1) Currently FieldFlagsMixin has a number of dynamic properties that are
only dynamic because they special-case checks for `is_reverse_object` --
I think it would be better if whatever sets `is_reverse_object` (only
`RelatedObject` I think?) simply switched the appropriate flags itself
(replacing a series of conditionals with polymorphic behavior).

 

Completly agreed. 

2) Currently `FieldFlagsMixin` does not only contain flags, it also
contains a definition of the `related_model` property. Perhaps it should
be renamed to simply `FieldMixin` if it is not limited to flags?

I'd argue whether a mixin is needed at all. At present, the mixin is only used on Field, GenericForeignKey and RelatedObject. RelatedObject will potentially be deleted; that leaves 2 classes. Every field in Django *except* for GenericForeignKey extends Field, so just having the attributes on Field with default values (just like all the other field attributes), and duplicating their definition on GenericForeignKey seems like a better approach to me. 

The only reasons to have a mixin or base class is if (a) there's complex logic, and/or (b) we have reason to believe that the functionality will be used elsewhere. If it's just a set of flags, then (a) isn't true; and I'm having difficulty thinking of any circumstance where (b) would be true, either.

3) I think it would be better if all the cardinality flags were present
on the base `FieldFlagsMixin` and defaulted to `None`. That would permit
many checks for e.g. `if field.has_relation and field.one_to_many` to be
safely replaced with the more concise `if field.one_to_many`. In general
I think `None` is a better choice than `raise AttributeError` for
standard flags that don't apply to a particular field.

 

Agreed. Given that our suggested mode of usage is filtering/list comprehensions, we should make it as easy as possible to implement those.

4) There is an open PR (originally authored by Anssi, recently updated
by Tim) to remove RelatedObject entirely:
https://github.com/django/django/pull/3757 -- I think that's a good PR
and should be merged for 1.8, but it will require some non-trivial
updates to the _meta PR once it is merged.

 

So... it's a race to commit, huh :-)

Seriously - I haven't looked closely at that patch, but the plumbing around RelatedObect has always been a bit messy; a cleanup would be great. There's certainly overlap in the two patches, and I agree it would be good to land both for 1.8 - if the API for related objects is going to change, better to do it in one pass.

Russ %-)

[Tom Christie]

One thing I'm unclear on: Does the new API contain any (public API) way of determining if a relationship has an associated through table?

Entirely possible that I'm being stupid and that's outside the scope of the API, but it's the one thing I see in Django REST framework's ModelSerializer '_meta' inspection code that's not obviously handled here.

Is it something that needs to be covered? (Of course if we're talking about 1.8 blockers I've no particular problem if the answer needs to be 'maybe but could come later')

Cheers,

Tom

[Russell Keith-Magee]

Hi Tom,

Well, it's covered (in that the capability exists), but that's a historical capability - it hasn't been through the formal documentation process.

The API we've documented has focussed on being able to discover the fields on a model. The fields themselves are (mostly) unchanged, and unless changes were necessary to support the cleanup of the discovery API, we haven't touched them.

So, something like:

[f for f in MyModel._meta.get_fields() if f.has_relation and f.many_to_many and not f.rel.through._meta.auto_created]

would give you all the m2m fields on this MyModel that have a m2m relation with a through model. The f.has_relation and f.many_to_many are new pieces of the API; f.rel.through._meta.auto_created is an old approach, but isn't formally documented.

I completely agree that in the longer term, these extra flags *should* be documented. There might even be an argument for them to be documented for 1.8 (if we're not changing anything - just formalising something that is already in use). 

My only hesitation is the overlap with the RelatedObject patch that Carl referred to. If that lands, some of these flags and mechanisms might change as a result of removing the RelatedObject model. I'd need to take a closer look at that patch to be able to work out if there's potential to box ourselves into a corner.

Yours,

Russ Magee %-)

[Carl Meyer]

On 12/22/2014 06:04 PM, Russell Keith-Magee wrote:
[snipped agreement on many things!]

> I'd argue whether a mixin is needed at all. At present, the mixin is only
> used on Field, GenericForeignKey and RelatedObject. RelatedObject will
> potentially be deleted; that leaves 2 classes. Every field in Django
> *except* for GenericForeignKey extends Field, so just having the attributes
> on Field with default values (just like all the other field attributes),
> and duplicating their definition on GenericForeignKey seems like a better
> approach to me.
>
> The only reasons to have a mixin or base class is if (a) there's complex
> logic, and/or (b) we have reason to believe that the functionality will be
> used elsewhere. If it's just a set of flags, then (a) isn't true; and I'm
> having difficulty thinking of any circumstance where (b) would be true,
> either.

This seems fine to me - at the moment it's not entirely clear to me what
determines whether something belongs on `FieldFlagsMixin` vs `Field`.

The one other reason I could think of to keep them separate is if we
want a smaller base class "documenting" (in code) precisely the required
API for something that wants to be a meta-API compatible field, without
any of the Django-ORM-specific stuff in Field (if you were building some
kind of non-Django-ORM but admin-compatible thing like Daniel's
impressive early Gmail prototype). But if that's what we want out of the
separation, I'm not sure we're quite there yet -- e.g. the `name`,
`attname`, `model` attributes at least appear to be required for a field
to participate in the meta API (at least they seem to be accessed in
`Options`), but they aren't currently mentioned on `FieldFlagsMixin`.

> 3) I think it would be better if all the cardinality flags were present
>> on the base `FieldFlagsMixin` and defaulted to `None`. That would permit
>> many checks for e.g. `if field.has_relation and field.one_to_many` to be
>> safely replaced with the more concise `if field.one_to_many`. In general
>> I think `None` is a better choice than `raise AttributeError` for
>> standard flags that don't apply to a particular field.
>>
>
> Agreed. Given that our suggested mode of usage is filtering/list
> comprehensions, we should make it as easy as possible to implement those.
>
> 4) There is an open PR (originally authored by Anssi, recently updated
>> by Tim) to remove RelatedObject entirely:
>> https://github.com/django/django/pull/3757 -- I think that's a good PR
>> and should be merged for 1.8, but it will require some non-trivial
>> updates to the _meta PR once it is merged.
>
>
> So... it's a race to commit, huh :-)

Guess so :-) Tim, are you planning to commit that patch or would you
like someone else to take a look first?

> Seriously - I haven't looked closely at that patch, but the plumbing around
> RelatedObect has always been a bit messy; a cleanup would be great. There's
> certainly overlap in the two patches, and I agree it would be good to land
> both for 1.8 - if the API for related objects is going to change, better to
> do it in one pass.

Yep, agreed.

Carl

[Tom Christie]

Thanks Russ, appreciate your time.

I've no great problem with keeping 'through' table API undocumented with 1.8, probably doesn't matter too much either way. Just flagging it up, but looks like 'no change' there, which is okay.

[Collin Anderson]

Odd. We should really save the "through" attribute on the m2m field itself. Maybe in 1.9...