Thoughts on Django Model attribute (descriptor) inheritance.

[Jarek Głowacki]

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

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

--
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/a7da90e7-a72c-482f-b195-eb196ae46e73%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Carlton Gibson]

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

[Ryan Hiebert]

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.

--

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/94d1f059-8cb2-435b-be52-66d3ae1a4721%40googlegroups.com.

[Carlton Gibson]

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