[Django] #27880: Use __set_name__ to replace some usages of contribute_to_class.

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: nobody
Charette |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords: python3.6
Triage Stage: | Has patch: 0
Someday/Maybe |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
If we were to define
[https://docs.python.org/3.6/reference/datamodel.html#object.__set_name__
__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.

--
Ticket URL: <https://code.djangoproject.com/ticket/27880>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Someday/Maybe => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:1>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev

(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Clifford Gama):

* owner: nobody => Clifford Gama
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:2>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Clifford Gama):

* has_patch: 0 => 1
* needs_docs: 0 => 1
* needs_tests: 0 => 1

Comment:

PR [https://github.com/django/django/pull/18621]
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:3>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Clifford Gama):

Forum discussion https://forum.djangoproject.com/t/set-name-
and-27880/35215
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:4>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Clifford Gama):

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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:5>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Clifford Gama):

[https://github.com/django/django/pull/18697 PR] addressing this ticket
for `Options`
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:6>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Clifford Gama):

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 results, some of which implement their
own `Options`. These would need to rename `contribute_to_class()` with
`__set_name__()` as this change is backward incompatible.
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:7>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Matthias Kestenholz):

It's not that easy to write library code which supports both code paths. I
"complained" to Loïc at DutH 2015 (or something) about the `get_query_set`
-> `get_queryset` rename. That one was really hard for third party app
maintainers to work around and support all possible combinations given
that people are supposed to use subclassing to customize behaviors.

This seems to be a similar case. It's nice that Python now supports
calling this method and that we could in principle use it for our own
purposes, but the explicitness of `contribute_to_class` might still have
some value. Especially given the fact that the change doesn't allow us to
remove a lot of code in the future, and given that it replaces an explicit
mechanism with a more implicit mechanism where people have to know more of
Python's internals to do what they want.

Also, you write that `contribute_to_class` is private API. It *is*
mentioned in the `docs/` folder of the Django repository so I'm not sure I
agree with that.

I'm quite sure the potential downsides of this change, especially the
impact to third party app developers are bigger than the upsides of using
new standard Python API.
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:8>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Thanks for chiming in Matthias!

I'll note that initial proposal of this ticket was not about
replacing/deprecating `contribute_to_class` which would effectively be
very disruptive to the ecosystem but to remove the `ModelBase.__new__`
magic that performs explicit calls to `contribute_to_class` by having
`__set_name__` call `contribute_to_class` instead.

To me this ticket kind of go hand-in-hand with `__init_subclasses__`
tickets #35827 and #34555 in the sense that it would be great to modernize
the implementation of `ModelBase.__new__` in a way that takes advantage of
these hooks while maintaining the developer facing interface and behavior.
It's easier said than done though and not a well understood problem from a
feasibility perspective.
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:9>

[Django]

#27880: Use __set_name__ to replace some usages of contribute_to_class.
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: python3.6 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Clifford Gama):

* owner: Clifford Gama => (none)
* status: assigned => new

Comment:

Thank you for clarifying the situation.

I'm not very experienced with the ecosystem, so I kind of suspected that
I'd done some (many) things wrong and was waiting for feedback on this. So
thanks for the feedback.

I don't think this is something I can take on, so I'm de-assigning myself
and closing related PRs. Great learning experience, though.
--
Ticket URL: <https://code.djangoproject.com/ticket/27880#comment:10>