Re: Inheritance of rel fields from base classes should not cause conflicts if the base classes involved are all abstract.
Thanks for the feedback. I've opened a ticket at http://code.djangoproject.com/ticket/15279. I did take a look at #14705, but though it is related topically, there didn't seem to be a way to exploit those changes for the effect I'm trying to achieve. My patch does still need improvement, docs, and tests; I plan to do that next week. Best, Stephen Burrows On Feb 10, 1:48 am, Russell Keith-Magee wrote: > On Wed, Feb 9, 2011 at 1:43 AM, Stephen Burrows > > > > > > > > > > wrote: > > Hi! I was going to submit this as a ticket, but I glanced over the patch > > submission guidelines once more and this seems to fall under the category of > > a "non-trivial change" that would require a design decision. The code > > samples/tracebacks can be viewed nicely formatted on github. [1] > > Assume the following models in `app`: > > from django.db import models > > > class Orange(models.Model): > > pass > > > class BaseClass(models.Model): > > field = models.ForeignKey(Orange) > > class Meta: > > abstract = True > > > class Parent1(BaseClass): > > class Meta: > > abstract=True > > > class Parent2(BaseClass): > > class Meta: > > abstract=True > > > class Child(Parent1, Parent2): > > pass > > Currently, this definition will raise the following errors during model > > validation: > > > Unhandled exception in thread started by > > > > Traceback (most recent call last): > > File "/django/core/management/commands/runserver.py", line 88, in > > inner_run > > self.validate(display_num_errors=True) > > File "/django/core/management/base.py", line 253, in validate > > raise CommandError("One or more models did not validate:\n%s" % > > error_text) > > django.core.management.base.CommandError: One or more models did not > > validate: > > app.child: Accessor for field 'field' clashes with related field > > 'Orange.child_set'. Add a related_name argument to the definition for > > 'field'. > > app.child: Accessor for field 'field' clashes with related field > > 'Orange.child_set'. Add a related_name argument to the definition for > > 'field'. > > Using the %(app_label)s_%(class)s_related syntax only makes things worse: > > > Unhandled exception in thread started by > > > > Traceback (most recent call last): > > File "/django/core/management/commands/runserver.py", line 88, in > > inner_run > > self.validate(display_num_errors=True) > > File "/django/core/management/base.py", line 253, in validate > > raise CommandError("One or more models did not validate:\n%s" % > > error_text) > > django.core.management.base.CommandError: One or more models did not > > validate: > > app.child: Accessor for field 'field' clashes with related field > > 'Orange.app_child_related'. Add a related_name argument to the definition > > for 'field'. > > app.child: Reverse query name for field 'field' clashes with related field > > 'Orange.app_child_related'. Add a related_name argument to the definition > > for 'field'. > > app.child: Accessor for field 'field' clashes with related field > > 'Orange.app_child_related'. Add a related_name argument to the definition > > for 'field'. > > app.child: Reverse query name for field 'field' clashes with related field > > 'Orange.app_child_related'. Add a related_name argument to the definition > > for 'field'. > > > Instead of causing errors, it seems like the field should only be inherited > > once from BaseClass. > > I don't think this is very controversial at all -- I can't see any > reason why BaseClass.field should be represented twice in Child's > field list. > > > My patch [1] handles this as follows: On each field > > instance, track the class it was originally declared for and the first > > non-abstract class that it shows up in. Then, when a field is being added to > > a class, check to make sure that it only gets added if it isn't a > > "duplicate". (The patch would incidentally also need to move the > > get_FIELD_display method declaration [2] into cls._meta.add_field.) If this > > is an acceptable idea, I would be happy to add tests and docs. If the > > general idea is acceptable, but the implementation is flawed, I would be > > happy to write patches using other methods that people may suggest; this was > > just what made sense to me. > > Thanks for the patch; if you want to make sure this isn't forgotten, > please open a ticket and attach your patch there. > > While you're in the ticket system, you might also want to take a look > at #14705. I haven't checked myself to be sure, but it's possible that > that patch might address this problem -- at the very least, it's > somewhat related. > > Yours, > Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this gr
Re: Inheritance of rel fields from base classes should not cause conflicts if the base classes involved are all abstract.
On Wed, Feb 9, 2011 at 1:43 AM, Stephen Burrows wrote: > Hi! I was going to submit this as a ticket, but I glanced over the patch > submission guidelines once more and this seems to fall under the category of > a "non-trivial change" that would require a design decision. The code > samples/tracebacks can be viewed nicely formatted on github. [1] > Assume the following models in `app`: > from django.db import models > > class Orange(models.Model): > pass > > class BaseClass(models.Model): > field = models.ForeignKey(Orange) > class Meta: > abstract = True > > class Parent1(BaseClass): > class Meta: > abstract=True > > class Parent2(BaseClass): > class Meta: > abstract=True > > class Child(Parent1, Parent2): > pass > Currently, this definition will raise the following errors during model > validation: > > Unhandled exception in thread started by > > Traceback (most recent call last): > File "/django/core/management/commands/runserver.py", line 88, in > inner_run > self.validate(display_num_errors=True) > File "/django/core/management/base.py", line 253, in validate > raise CommandError("One or more models did not validate:\n%s" % > error_text) > django.core.management.base.CommandError: One or more models did not > validate: > app.child: Accessor for field 'field' clashes with related field > 'Orange.child_set'. Add a related_name argument to the definition for > 'field'. > app.child: Accessor for field 'field' clashes with related field > 'Orange.child_set'. Add a related_name argument to the definition for > 'field'. > Using the %(app_label)s_%(class)s_related syntax only makes things worse: > > Unhandled exception in thread started by > > Traceback (most recent call last): > File "/django/core/management/commands/runserver.py", line 88, in > inner_run > self.validate(display_num_errors=True) > File "/django/core/management/base.py", line 253, in validate > raise CommandError("One or more models did not validate:\n%s" % > error_text) > django.core.management.base.CommandError: One or more models did not > validate: > app.child: Accessor for field 'field' clashes with related field > 'Orange.app_child_related'. Add a related_name argument to the definition > for 'field'. > app.child: Reverse query name for field 'field' clashes with related field > 'Orange.app_child_related'. Add a related_name argument to the definition > for 'field'. > app.child: Accessor for field 'field' clashes with related field > 'Orange.app_child_related'. Add a related_name argument to the definition > for 'field'. > app.child: Reverse query name for field 'field' clashes with related field > 'Orange.app_child_related'. Add a related_name argument to the definition > for 'field'. > > Instead of causing errors, it seems like the field should only be inherited > once from BaseClass. I don't think this is very controversial at all -- I can't see any reason why BaseClass.field should be represented twice in Child's field list. > My patch [1] handles this as follows: On each field > instance, track the class it was originally declared for and the first > non-abstract class that it shows up in. Then, when a field is being added to > a class, check to make sure that it only gets added if it isn't a > "duplicate". (The patch would incidentally also need to move the > get_FIELD_display method declaration [2] into cls._meta.add_field.) If this > is an acceptable idea, I would be happy to add tests and docs. If the > general idea is acceptable, but the implementation is flawed, I would be > happy to write patches using other methods that people may suggest; this was > just what made sense to me. Thanks for the patch; if you want to make sure this isn't forgotten, please open a ticket and attach your patch there. While you're in the ticket system, you might also want to take a look at #14705. I haven't checked myself to be sure, but it's possible that that patch might address this problem -- at the very least, it's somewhat related. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Inheritance of rel fields from base classes should not cause conflicts if the base classes involved are all abstract.
Hi! I was going to submit this as a ticket, but I glanced over the patch submission guidelines once more and this seems to fall under the category of a "non-trivial change" that would require a design decision. The code samples/tracebacks can be viewed nicely formatted on github. [1] Assume the following models in `app`: from django.db import models class Orange(models.Model): pass class BaseClass(models.Model): field = models.ForeignKey(Orange) class Meta: abstract = True class Parent1(BaseClass): class Meta: abstract=True class Parent2(BaseClass): class Meta: abstract=True class Child(Parent1, Parent2): pass Currently, this definition will raise the following errors during model validation: Unhandled exception in thread started by > Traceback (most recent call last): File "/django/core/management/commands/runserver.py", line 88, in inner_run self.validate(display_num_errors=True) File "/django/core/management/base.py", line 253, in validate raise CommandError("One or more models did not validate:\n%s" % error_text) django.core.management.base.CommandError: One or more models did not validate: app.child: Accessor for field 'field' clashes with related field 'Orange.child_set'. Add a related_name argument to the definition for 'field'. app.child: Accessor for field 'field' clashes with related field 'Orange.child_set'. Add a related_name argument to the definition for 'field'. Using the %(app_label)s_%(class)s_related syntax only makes things worse: Unhandled exception in thread started by > Traceback (most recent call last): File "/django/core/management/commands/runserver.py", line 88, in inner_run self.validate(display_num_errors=True) File "/django/core/management/base.py", line 253, in validate raise CommandError("One or more models did not validate:\n%s" % error_text) django.core.management.base.CommandError: One or more models did not validate: app.child: Accessor for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'. app.child: Reverse query name for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'. app.child: Accessor for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'. app.child: Reverse query name for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'. Instead of causing errors, it seems like the field should only be inherited once from BaseClass. My patch [1] handles this as follows: On each field instance, track the class it was originally declared for and the first non-abstract class that it shows up in. Then, when a field is being added to a class, check to make sure that it only gets added if it isn't a "duplicate". (The patch would incidentally also need to move the get_FIELD_display method declaration [2] into cls._meta.add_field.) If this is an acceptable idea, I would be happy to add tests and docs. If the general idea is acceptable, but the implementation is flawed, I would be happy to write patches using other methods that people may suggest; this was just what made sense to me. Best, Stephen Burrows [1] http://gist.github.com/815337 [2] http://code.djangoproject.com/browser/django/trunk/django/db/models/fields/__init__.py#L238 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.