Opened 8 years ago

Last modified 4 weeks ago

#27880 assigned Cleanup/optimization

Use __set_name__ to replace some usages of contribute_to_class.

Reported by: Simon Charette Owned by: Clifford Gama
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: python3.6
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If we were to define __set__name__ calling contribute_to_class() on models.Field, Manager and Options the following things could be done:

  1. ModelBase.add_to_class calls could be replaced by simple setattr(cls, name, value).
  2. Options.contribute_to_class could be inlined in Options.__set_name__.
  3. Manager.contribute_to_class could be inlined in Manager.__set_name__.
  4. There might be a way to inline Field.contribute_to_class by dealing with its private_only flag somehow.

Change History (7)

comment:1 by Mariusz Felisiak, 4 years ago

Triage Stage: Someday/MaybeAccepted

comment:2 by Clifford Gama, 2 months ago

Owner: changed from nobody to Clifford Gama
Status: newassigned

comment:3 by Clifford Gama, 7 weeks ago

Has patch: set
Needs documentation: set
Needs tests: set

comment:5 by Clifford Gama, 4 weeks ago

Closed https://github.com/django/django/pull/18621 as the work on Field.contribute_to_class() was not production-ready, and as there was no feedback on the implementation in forum discussion.

comment:6 by Clifford Gama, 4 weeks ago

PR addressing this ticket for Options

comment:7 by Clifford Gama, 4 weeks ago

Note that while Options.contribute_to_class() is private API, searching /(?-i)Options/ django.db "def contribute_to_class" -path:django/db path:*py in GitHub gives about 240 matches, some of which implement their own Options. These would need to rename contribute_to_class() with __set_name__() as this change is backward incompatible.

Last edited 4 weeks ago by Clifford Gama (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top