Thoughts on Django Model attribute (descriptor) inheritance.

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Thoughts on Django Model attribute (descriptor) inheritance.

Jarek Glowacki
Hi friends,

I've raised a ticket here:
https://code.djangoproject.com/ticket/30427#ticket
Has an associated PR and all.

Looking for some experts in this area to eyeball and drop some thoughts/opinions, or even better, knowledge as to why we're doing things this way presently.
I feel Django does class inheritance wrong at the moment -- when building a Model, it refuses to staple on a class attribute if an ancestor/mixin has already defined that attribute (with a non-falsey value). One could call it reverse-MRO behaviour.

It looks like the intention in the past was to stop attributes or field definitions from stomping on methods defined against the same class that have the same name, but if anything, I'd rather have this raise a proper warning if it's problematic, or silently stomp if it's not. Having it not stomp results in some very unpythonic behaviour.
Have a read of my ticket/PR for further context.

Looking forward to getting this fixed, so I can bubble the fix up to django-model-utils, and then to my production code. ^.^

Cheers,
Jarek

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/a7da90e7-a72c-482f-b195-eb196ae46e73%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Django Model attribute (descriptor) inheritance.

Kye Russell
I don’t know enough to speak on the reasoning behind the current implementation, but from the perspective of developer experience, I’ve run into this a few times, and the current behaviour has felt jarring and unpythonic.


From: Jarek Głowacki [hidden email]
Reply: [hidden email] [hidden email]
Date: 15 May 2019 at 12:11:15 pm
To: Django developers (Contributions to Django itself) [hidden email]
Subject:  Thoughts on Django Model attribute (descriptor) inheritance.

Hi friends,

I've raised a ticket here:
https://code.djangoproject.com/ticket/30427#ticket
Has an associated PR and all.

Looking for some experts in this area to eyeball and drop some thoughts/opinions, or even better, knowledge as to why we're doing things this way presently.
I feel Django does class inheritance wrong at the moment -- when building a Model, it refuses to staple on a class attribute if an ancestor/mixin has already defined that attribute (with a non-falsey value). One could call it reverse-MRO behaviour.

It looks like the intention in the past was to stop attributes or field definitions from stomping on methods defined against the same class that have the same name, but if anything, I'd rather have this raise a proper warning if it's problematic, or silently stomp if it's not. Having it not stomp results in some very unpythonic behaviour.
Have a read of my ticket/PR for further context.

Looking forward to getting this fixed, so I can bubble the fix up to django-model-utils, and then to my production code. ^.^

Cheers,
Jarek
--
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 [hidden email].
To post to this group, send email to [hidden email].
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/a7da90e7-a72c-482f-b195-eb196ae46e73%40googlegroups.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 [hidden email].
To post to this group, send email to [hidden email].
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/CANK-ykm7Ch3f%3D%3D4qn0iaj%2BMy9P2BO-YvSdk4Ygs4gPbg5-xmtg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Django Model attribute (descriptor) inheritance.

Carlton Gibson-3
In reply to this post by Jarek Glowacki
Hi All

Can I ask folks to have a look at this please? 

Ticket is: https://code.djangoproject.com/ticket/30427
PR is: https://github.com/django/django/pull/11337

The original change that introduced the current behaviour is 
https://github.com/django/django/pull/6491/files#diff-bf776a3b8e5dbfac2432015825ef8afeR699

Question: What was the reason for this comment: 

# Don't override classmethods with the descriptor. This means that
# if you have a classmethod and a field with the same name, then
#such fields can't be deferred (we don't have a check for this).

In case it rings any bells...

PR there was called:

    "Replaced dynamic classes with non-data descriptors for deferred instance loading."

Related mailing list discussion: https://groups.google.com/forum/#!msg/django-developers/BDAlTyJwQeY/Fh2Qy07cAQAJ

Thanks. 
Carlton

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/94d1f059-8cb2-435b-be52-66d3ae1a4721%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Django Model attribute (descriptor) inheritance.

Ryan Hiebert-2
I'm not sure what the reasoning was, but it does ring some bells for me, as I think this sounds like the case of the of the issue discovered in https://code.djangoproject.com/ticket/28198. Hopefully that's a correct connection, and I'm not sending you chasing something irrelevant to your current task.

On Fri, Jun 21, 2019 at 8:39 AM Carlton Gibson <[hidden email]> wrote:
Hi All

Can I ask folks to have a look at this please? 


Question: What was the reason for this comment: 

# Don't override classmethods with the descriptor. This means that
# if you have a classmethod and a field with the same name, then
#such fields can't be deferred (we don't have a check for this).

In case it rings any bells...

PR there was called:

    "Replaced dynamic classes with non-data descriptors for deferred instance loading."


Thanks. 
Carlton

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/94d1f059-8cb2-435b-be52-66d3ae1a4721%40googlegroups.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 [hidden email].
To post to this group, send email to [hidden email].
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/CABpHFHR-mhQGneh-bjUV6Oa_DubVVDOC2jc_CWaBoR6LPVdZTg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Django Model attribute (descriptor) inheritance.

Carlton Gibson-3
Hi Ryan, 

That does look related, yes. 

On Friday, 21 June 2019 15:47:53 UTC+2, Ryan Hiebert wrote:
Hopefully that's a correct connection, and I'm not sending you chasing something irrelevant to your current task.

TBH any pointers are handy at this stage in the game. 🙂

Thanks! 

(Still interested in hearing what original thoughts were if anyone _can_ dig them up in the grey-matter...)

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/c35f0b82-5b82-4cfb-bd98-4756ee93dea9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.