Thanks for this! Agree with your points and would be happy to add that documentation to the PR
On your point about accessing `_meta` in `__init_subclass__`: in the interest of avoiding overcomplication I didn't bring it up in issue #34555, but now that the cat is out of the bag... :) I separately wrote a further change and initial unittests that would allow `_meta` to be added/modified in `__init_subclass__` - please see diff (relative to changes proposed for #34555) here: https://github.com/hottwaj/django/pull/1/files My plan was/is to submit this as a separate issue/PR if the changes in #34555 get the green light. Some notes on implementation: - I could not figure out how to change `ModelBase.__new__` to call `super().__new__` and set the `_meta` attr such that `__init_subclass__` could access the `_meta` attribute **without** breaking existing unittests. Maybe someone more familiar with it than me can figure it out though I think this is mainly a design feature of current implementation - instead, I tweaked `ModelBase.__new__` to pass a kwarg `cls_meta` to super().__new__` which is absorbed by `AltersData.__init_subclass__`. Probably a new (otherwise blank) method `ModelBase.__init_subclass__` would be a better place to absorb the new `cls_meta` kwarg instead. - with this approach you can see in the unittests that if a user wants to add/edit `_meta` in their implementation of `__init_subclass__` some extra setup is needed to create a blank `_meta` if one is not present. Probably this boilerplate should go in a classmethod or as preparatory stuff in `ModelBase.__new__` I think this change is also fairly unobtrusive, except that existing packages/users that make use of `__init_subclass__` would have to take care to pass through to `super().__init_subclass__` the new `cls_meta` kwarg so that it is available in a multiple inheritance scenario. Either way, you are right that `__init_subclass__` will not have access to model fields defined in the "usual" way (unless a similar approach of passing them as a kwarg is used), which will probably be surprising Thanks! On Tuesday, May 16, 2023 at 7:17:19 PM UTC+1 charettes wrote: > Just wanted to publicly +1 here as well. I was skeptical that we could add > support for it without invasive changes at first but it seems to be > relatively straightforward to support. > > One ask I would add is that we explicitly document what is support and > what isn't. For example, at the time `__init_subclass__` is called one > should not expect `_meta` or any other subclass fields to be available and > that even when calling `super().__init_subclass__`. That might come as a > surprise to users that want to do anything to a trivial field addition > (e.g. perform model introspection). For non-trivial use cases a > class_prepared signal seems like it's still the best way to run code once > the class is fully initialized. > > Changing these expectations could be done by moving most of the > ModelBase.__new__ logic to Model.__init_subclass__ but this would require a > massive re-enginering of meta programming logic that is remain unchanged > for years. > > Le vendredi 12 mai 2023 à 09:38:04 UTC-4, Adam Johnson a écrit : > >> +1 from me. As Simon covered on the ticket, the change is small. Making >> Django classes support __init_subclass__ might unlock some nice dynamic >> field patterns. >> >> On Thu, May 11, 2023 at 12:47 PM hottwaj <jonathan...@gmail.com> wrote: >> >>> Hi there, I opened the above ticket and submitted a PR with fix and test >>> added. I was asked to bring the issue here for wider review before the >>> ticket is re-opened (if that is what people agree to do) >>> >>> For reference, links to the ticket and PR are: >>> https://code.djangoproject.com/ticket/34555 >>> https://github.com/django/django/pull/16849 >>> >>> The issue raised is that current implementation of ModelBase.__new__ >>> prevents use of __init_subclass__ on a Model to add model fields >>> >>> e.g. the code listed at the end of this email does not currently work >>> (the PR fixes this). >>> >>> Currently the same result could be achieved by i) writing a new >>> metaclass e.g. BaseBookModelMeta or ii) using a class decorator where >>> cls.add_to_class(field) is called. >>> >>> Using __init_subclass__ is advised as a simpler alternative to writing a >>> metaclass to customise class creation, hence this PR. >>> >>> Hope that makes sense and appreciate any feedback. Thanks! >>> >>> >>> class BaseBookModel(models.Model): >>> class Meta: >>> abstract = True >>> >>> def __init_subclass__(cls, author_cls, **kwargs): >>> super().__init_subclass__(**kwargs) >>> cls.author = models.ForeignKey(author_cls, >>> on_delete=models.CASCADE) >>> >>> class Author(models.Model): >>> name = models.CharField(max_length=256, unique=True) >>> >>> class Book(BaseBookModel, author_cls=Author): >>> pass >>> >>> -- >>> 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 view this discussion on the web visit >>> https://groups.google.com/d/msgid/django-developers/015a5798-d084-4afb-b800-e83154301ec7n%40googlegroups.com >>> >>> <https://groups.google.com/d/msgid/django-developers/015a5798-d084-4afb-b800-e83154301ec7n%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- 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/15034072-a111-47a7-9032-902b5fde2d33n%40googlegroups.com.
