Re: Extending the checks for related-name clashes

2020-09-07 Thread Adam Johnson
>
> I believe the technical term of this is "a mess".
>

Looks like it!

I can't think of any reason why this inconsistency would be desirable.
Adding more checks is normally fine since they can be disabled by users who
want/rely on the behaviour.

On Sun, 6 Sep 2020 at 13:54, Shai Berger  wrote:

> On Sun, 6 Sep 2020 12:01:34 +0100
> Adam Johnson  wrote:
>
> > I think it would be acceptable to make related name clashes a check
> > Error. I'm guessing you couldn't find any justification for why the
> > check doesn't currently do this?
> >
>
> I didn't find any, though I didn't look too hard.
>
> On the other hand, I found that the behavior changes depending on the
> target model being a proxy model (or rather, on the model defining the
> property being a proxy model).
>
> That is, with
>
> class A(models.Model):
> @property
> def related(self):
> return 15
>
>
> class B(models.Model):
> a = models.ForeignKey(A, related_name='related',
>   on_delete=models.CASCADE)
>
>
> class C(A):
> class Meta:
> proxy = True
>
> @property
> def pro_related(self):
> return 17
>
>
> class D(models.Model):
> c = models.ForeignKey(C, related_name='pro_related',
>   on_delete=models.CASCADE)
>
>
> For instances a and c of A and C respectively,
> a.related is a RelatedManager
> c.pro_related is 17
>
> This happens because reverse-accessors are promoted to the concrete
> model -- the accessor attribute is set on A, which means it overrides
> whatever was defined on A, but can be overridden by A's subclasses.
>
> I believe the technical term of this is "a mess".
>
> Shai.
>
> --
> 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-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/20200906155425.17c1d7f8.shai%40platonix.com
> .
>


-- 
Adam

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM2s6gk0XFZTQ58zhcOx-2N%3Do7aS_XSpr3XVFxb1X%3DbMNQ%40mail.gmail.com.


Re: Extending the checks for related-name clashes

2020-09-06 Thread Shai Berger
On Sun, 6 Sep 2020 12:01:34 +0100
Adam Johnson  wrote:

> I think it would be acceptable to make related name clashes a check
> Error. I'm guessing you couldn't find any justification for why the
> check doesn't currently do this?
> 

I didn't find any, though I didn't look too hard.

On the other hand, I found that the behavior changes depending on the
target model being a proxy model (or rather, on the model defining the
property being a proxy model).

That is, with

class A(models.Model):
@property
def related(self):
return 15


class B(models.Model):
a = models.ForeignKey(A, related_name='related',
  on_delete=models.CASCADE)


class C(A):
class Meta:
proxy = True

@property
def pro_related(self):
return 17


class D(models.Model):
c = models.ForeignKey(C, related_name='pro_related',
  on_delete=models.CASCADE)


For instances a and c of A and C respectively,
a.related is a RelatedManager
c.pro_related is 17

This happens because reverse-accessors are promoted to the concrete
model -- the accessor attribute is set on A, which means it overrides
whatever was defined on A, but can be overridden by A's subclasses.

I believe the technical term of this is "a mess".

Shai.

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20200906155425.17c1d7f8.shai%40platonix.com.


Re: Extending the checks for related-name clashes

2020-09-06 Thread Adam Johnson
I think it would be acceptable to make related name clashes a check Error.
I'm guessing you couldn't find any justification for why the check doesn't
currently do this?

On Sun, 6 Sep 2020 at 11:31, Shai Berger  wrote:

> Hi all,
>
> When you define a related field on a model -- a ForeignKey etc -- it
> usually adds its related-name as a backwards-accessor on the related
> model. These related names are checked for clashes against other fields
> and other related-names. But they are not checked for clashes against
> other attributes of the remote model, like methods and properties.
>
> A clash with another field (or accessor) is considered an error. I
> think a clash with a property should be at least a warning. Currently
> it is accepted silently (and AFAICT, the related-name takes
> precedence, that is, the property is effectively deleted).
>
> Does anybody think this is the desired behavior?
>
> Thanks,
> Shai.
>
> --
> 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-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/20200906133146.4449641c.shai%40platonix.com
> .
>


-- 
Adam

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM2ANYnO1j4MsC9LAqik7uXk-2n%3DGXhu0_TmGjkVToQ4ow%40mail.gmail.com.


Extending the checks for related-name clashes

2020-09-06 Thread Shai Berger
Hi all,

When you define a related field on a model -- a ForeignKey etc -- it
usually adds its related-name as a backwards-accessor on the related
model. These related names are checked for clashes against other fields
and other related-names. But they are not checked for clashes against
other attributes of the remote model, like methods and properties.

A clash with another field (or accessor) is considered an error. I
think a clash with a property should be at least a warning. Currently
it is accepted silently (and AFAICT, the related-name takes
precedence, that is, the property is effectively deleted).

Does anybody think this is the desired behavior?

Thanks,
Shai.

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20200906133146.4449641c.shai%40platonix.com.