Issues with replacing contribute_to_class with __set_name__ (#27880)

Django Internals ORM
1

Hello everyone :wave:

I am currently working on #27880: Use __set_name__ to replace some usages of contribute_to_class and I would like to open the following points for discussion:

Errors raised in typeobject’s __set_name__.

Prior to Python 3.12 55c99d9, exceptions raised in typeobject’s __set_name__ were wrapped by a RuntimeError exception. If I understood #27800 correctly and my implementation in PR is okay, then Fields, Managers and Options should now be included as attributes in type.__new__() call in ModelBase. This poses a problem because the following:

class AutoFieldMixin:
    def __set_name__(self, cls, name):
        if cls._meta.auto_field:
            raise ValueError(
                "Model %s can't have more than one auto-generated field."
                % cls._meta.label
            )
        super().__set_name__(cls, name)
        cls._meta.auto_field = self

propagates a RuntimeError along with the expected AttributeError, leading to the following test failing in Python 3.10 and 3.11:

class MultipleAutoFieldsTests(TestCase):
    def test_multiple_autofields(self):
        msg = (
            "Model invalid_models_tests.MultipleAutoFields can't have more "
            "than one auto-generated field."
        )
        with self.assertRaisesMessage(ValueError, msg):
            class MultipleAutoFields(models.Model):
                auto1 = models.AutoField(primary_key=True)
                auto2 = models.AutoField(primary_key=True)

I’m not sure how to address this issue effectively and would appreciate any insights or recommendations.

Issue with __set_name__ in Non-Model Classe definitions:

If a Field instance is used as an attribute in a non-model class definition, such as an output_field in a models.expressions.Func subclass, __set_name__ will be triggered when the class is created. This can lead to crashes because __set_name__ expects the class to have a _meta attribute, which non-model classes don’t have.

In practice, Django itself uses output_field in custom expressions to define the type of the result for expressions like Func, F, and ExpressionWrapper. Most of these fields don’t override __set_name__, so they aren’t affected. However, for those fields that do override __set_name__, we might need to consider how to handle this.

My current approach involves adding a duck-typing check (ensuring _meta is present) and repeating it in every Field subclass that overrides __set_name__ and is used by Django in a non-model class definition.

I’m wondering

  1. Should we apply this check across all Field subclasses that override __set_name__ for consistency, or
  2. Should the use of Fields in non-model classes be treated as an undocumented or unsupported edge case? Since the issue seems rare—most expressions using output_field don’t override __set_name__—we could potentially leave this edge case for users to manage themselves if needed.

Any alternative approaches, thoughts on whether this should be documented as an edge case, officially unsupported, or handled differently would be very helpful!

Thanks.

1 Like
2

I’m not sure if I can answer your question; it seems to me that detecting whether we’re on a model or somewhere else inside __set_name__ is somewhat more involved and magic than what he nave now, where we can know what we should do since we’re explicitly called in contribute_to_class (without Python magic)

The other thing is that contribute_to_class had only been mentioned only in some release notes when the Trac ticket was initially opened, but was somewhat documented here a few years later: Fixed #31124 -- Fixed setting of get_FOO_display() when overriding in… · django/django@29c126b · GitHub . The ticket itself was accepted even later than that, but since the method is documented now you’d probably also need a deprecation strategy.

I hope I’m not too discouraging here!

3

Hello @matthiask,

Thank you for sharing your concerns—I completely understand and share them, especially regarding the transition from Field.contribute_to_class (#35772). I added a deprecation strategy for Field.contribute_to_class(), which I hope helps in addressing that aspect.

As for your second concern: __set_name__ can be invoked similarly to contribute_to_class when necessary. The challenge isn’t whether it will be called, but rather how it should behave when it’s invoked by “Python magic” in a non-model class creation, and whether we should account for fields being defined in such classes. Should we expect fields to be defined in non-model classes?

And no, you’re not discouraging in any way—I appreciate the feedback!
Thanks.