Re: [Django] #34555: ModelBase metaclass implementation prevents addition of model fields via __init_subclass__
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):
* status: new => closed
* resolution: => wontfix
Comment:
Adapting `ModeBase.__new__` to play better with `__init_subclass__` and
possibly [https://code.djangoproject.com/ticket/24313 pave the way for
some deprecation] is one thing but maintaining backward compatibility (or
at least providing a bullet proof deprecation path) with the hundreds of
third-party applications that rely on the current implementation is-far
from trivial. For your particular use case to work, that is `cls._meta`
being already present at `__init_subclass__` time, the whole concept of
`add_to_class` would have to be thrown away as `object.__new__` (aka
`super().__new__` in `ModeBase.__new__`) triggers `__init_subclass__`.
That's also the reason why `contribute_to_class` is still relevant today
even if recent versions of Python introduced a `__set_name__` hook that is
a strong contender to replace it.
If you can demonstrate that there is a way to achieve such thing (e.g. a
PoC PR) and that there are strong enough benefits to warrant the burden
this will incur on third party application that have been using `__new__`
for years and might need to be adjusted then I believe you might have a
stronger case than by asking for this request to be adjusted in its
current form.
I'll be closing as ''wontfix'' until it can be demonstrated that this is
somewhat technically achievable at least.
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by hottwaj):
Thanks, I completely agree that this change should not break the existing
API in any way.
Please take a look at proposed fix and test added in this PR:
https://github.com/django/django/pull/16849
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:4>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:5>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
init_subclass | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by hottwaj):
* status: closed => new
* resolution: wontfix =>
Comment:
As I've added a PoC PR and in case it is needed to continue the process
I'm reopening - hope I'm not generating too much noise :)
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:6>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Component: Database layer | Version: 4.2
(models, ORM) |
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
I appreciate you'd like to reopen the ticket, but please
[https://docs.djangoproject.com/en/stable/internals/contributing/triaging-
tickets/#closing-tickets follow the triaging guidelines with regards to
wontfix tickets] and take this to DevelopersMailingList.
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:7>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Thanks for the PoC, the PR is definitely less invasive than I initially
envisioned so my main concerns are addressed. I do agree with Mariusz that
this should be brought to the scrutiny of the mailing list though to
gather more feedback on the approach and evaluate the appetite of the
community for such a change.
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:8>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by hottwaj):
Great, thanks Simon & Mariusz. I've sent an email to the mailing list as
requested, think it is just waiting approval, so will wait to hear on
further feedback there. Cheers!
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:9>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Related to is #27880 ''Use `__set_name__` to replace some usages of
contribute_to_class'', I think
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:10>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Carlton Gibson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:11>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
init_subclass | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Replying to [comment:10 Carlton Gibson]:
> Related too is #27880 ''Use `__set_name__` to replace some usages of
contribute_to_class'', I think
Indeed, if #27880 gets added then this might be fixed, thanks to
`Options.__set_name__()`, `_meta` will be added when the class is first
created.
I say *might* because as the PR for #27880 currently stands,
`Field.contribute_to_class()` will still be called after the class is
first created, because `Field.contribute_to_class()` is semi-private API
and has to be deprecated rather than replaced.
Regardless of whether `Field.contribute_to_class()` is deprecated for
`Field.__set_name__()`, if
[https://github.com/django/django/pull/18621/commits/f5fa7b4986e0ef2c994dffc95c003f55bf7dbe8c
f5fa7b4] is added allowing Options to be added on class creation (i.e.
`type.__new__()` in `ModelBase`) then the following would work
{{{#!python
from django.db.models import Model, ForeignKey, CASCADE, CharField
class BaseBookModel(Model):
class Meta:
abstract = True
def __init_subclass__(cls, author_model_cls: Type[Model], **kwargs,):
super().__init_subclass__(**kwargs)
author = ForeignKey(author_model_cls, on_delete = CASCADE)
# This will work because `_meta` is available when init-subclass
is called
author.contribute_to_class(cls, 'author')
# author.__set_name__(cls, 'author') if #27880 is landed for
Fields
class Author(Model):
name = CharField(max_len = 256, unique = True)
class Book(BaseBookModel, author_model_cls = Author):
pass
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:12>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: ModelBase | Triage Stage:
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Someday/Maybe
Comment:
Thought I should link [https://github.com/django/django/pull/18697 this
new PR] on #27880 as it would address this ticket and #35872.
Also triaging as "Someday/Maybe" since #27880 is "Accepted" and #35872 is
"Someday/Maybe"
--
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:13>
Django
__init_subclass__
-------------------------------------+-------------------------------------
Reporter: hottwaj | Owner: nobody
(models, ORM) |
Severity: Normal | Resolution:
init_subclass | Someday/Maybe
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Clifford Gama):
Ticket URL: <https://code.djangoproject.com/ticket/34555#comment:14>