Proposal: Allow ManyToMany using a intermediary table to be defined as symmetrical

[Nadège Michel]

Hello,

 

We use self.referencing ManyToMany relationships with intermediate tables in our work project 

and I was wondering why we had to create ourselves the reverse link when we need the relationship to be symmetrical.

I looked at the 'symmetrical' attribute documentation and though we should just set it to True instead of False, 

but you may know that it triggers the error "fields.E332 Many-to-many fields with intermediate tables must not be symmetrical.".

I searched for a corresponding existing ticket a found this one which is kind of related https://code.djangoproject.com/ticket/9475

And you can see in the PR some discussion about the check https://github.com/django/django/pull/8981#discussion_r247946460

Thanks to the work of Collin Anderson in the previous PR I think we can now remove that check.

Questions I had:

• retro compatibility?

As the current behavior forces the user to set 'symmetrical=False', the change is retro-compatible.

• use 'through_defaults' when creating both objects or define a new 'through_reverse_defaults' to give different values for each row? 

if you want symmetrical relationship, both objects should have the same values 

so I chose to use 'through_defaults' for both objects. Which was also what was done in the #8981 PR in the first place.

I have a patch with theses changes:

• removed the check 
• removed tests for that check
• added tests (tests/m2m_through, tests/m2m_recursive)
• updated documentation (may need a bit more work)
• added back the use of 'through_defaults' of #8981 in the:

if self.symmetrical:  

self  ._add_items(...) 

Tests suite runs fine.

Any thoughts on this design change / new feature?  

I'll be happy to create a ticket and submit my patch for reviews :) 

[Aymeric Augustin]

Hello Nadège,

While I'm not an expert of this area, your proposal makes sense to me. It looks like a reasonable extension of previous work. You can go ahead with a ticket and a pull request.

Best regards,

-- 

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 django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3e2f799d-3444-407b-bc81-523c3f55ec0b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Asif Saif Uddin]

After reading your reasoning and code it seems logical for me. Let's create a pr for more input.

[michel...@gmail.com]

Thank you for the feedback :)

For reference:

Ticket https://code.djangoproject.com/ticket/30421

Pull request https://github.com/django/django/pull/11298

-- 

Aymeric.

To unsubscribe from this group and stop receiving emails from it, send an email to django-d...@googlegroups.com.

[Collin Anderson]

Hi Nadège,

Sorry for the delay. Yeah, when I was working on PR 8981, I ran into the symmetrical case and wasn't sure what the behavior should be, so I left the check in place to not allow the behavior. I asked my self the same question you did about possibly needing "through_reverse_defaults" and didn't have answer. I also didn't have a use-case for symmetrical with intermediary/through. I wasn't sure about other unintended consequences and just wanted to get the rest of the change out the door.

If you think "both objects should have the same value" then I'd say go for it. I suppose it does make sense considering it's supposed to be symmetrical as you said. If later people want through_reverse_defaults we could always add it later.

Thanks,

Collin