[Django] #35542: BoundField's label and help_text and renderer should be properties not class members

32 views
Skip to first unread message

Django

unread,
Jun 20, 2024, 8:39:16 AM6/20/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: | Owner: Christophe Henry
Christophe Henry |
Type: | Status: assigned
Uncategorized |
Component: Forms | Version: 5.0
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`BoundField` exposes
[https://github.com/django/django/blob/main/django/forms/boundfield.py#L23
label]
[https://github.com/django/django/blob/main/django/forms/boundfield.py#L27
help_text] and
[https://github.com/django/django/blob/main/django/forms/boundfield.py#L28
renderer], all of them are copies of underlying members of `Field` and
`Form`.

This is a bit misleanding since you can modify `Field`s member in `Form`'s
`__init__` [https://docs.djangoproject.com/en/5.0/ref/forms/fields
/#fields-which-handle-relationships like documented here] but any
modification of `help_text`, `label` or `renderer` after
`super().__init__` won't be reflected on the bound which can leand to
incomprehensible behavior in templates
--
Ticket URL: <https://code.djangoproject.com/ticket/35542>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jun 21, 2024, 3:47:36 PM6/21/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* resolution: => needsinfo
* status: assigned => closed
* type: Uncategorized => Cleanup/optimization
* version: 5.0 => dev

Comment:

Hello again Christophe Henry! Thank you for your report.

We would need more information to understand your use case. The
documentation you linked about "Fields which handle relationships" refers
to redefining a field's queryset, not the label/help_text/renderer/etc.

With the information provided so far, I cannot determine whether this is a
very specific issue arising from a niche use case or something that
applies to the broader ecosystem. Django is a framework designed to
provide robust and reliable solutions for common scenarios. Therefore,
before accepting any changes to well-established and mature code, it is
crucial to demonstrate clear benefits from the proposed modification and
ensure there are no risks of breaking existing functionality.

Specifically, we would need a small Django test project or a failing test
case that shows the code path or use case that needs fixing. I'll close as
`needsinfo` in the meantime.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:1>

Django

unread,
Jun 27, 2024, 7:04:12 AM6/27/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Christophe Henry):

Replying to [comment:1 Natalia Bidart]:

Hello Natalia!

Well the documentation I linked features this as the first example:

{{{#!python
class FooMultipleChoiceForm(forms.Form):
foo_select = forms.ModelMultipleChoiceField(queryset=None)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields["foo_select"].queryset = ...
}}}

The same way, you can do this, for instance:

{{{#!python
class FooMultipleChoiceForm(forms.Form):
foo_select = forms.CharField()

def __init__(self, user: User, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields["foo_select"].help_text = f"Indicate
{user.first_name}'s pet name"
}}}

But if you do this, you won't get the result you expect when rendering
form with `{{ form }}` since what will be rendered is
`self["foo_select"].help_text.` (the associated `BoundField`) and not
`self.fields["foo_select"].help_text`. In order for the help text to
correctly be rendered, you need to write:

{{{#!python
class FooMultipleChoiceForm(forms.Form):
foo_select = forms.CharField()

def __init__(self, user: User, *args, **kwargs):
super().__init__(*args, **kwargs)
# Not self.field
self["foo_select"].help_text = f"Indicate {user.first_name}'s pet
name"
}}}

This is confusing and is inconsistant with the `queryset` example of the
documentation for anyone who's not very familiar with Django's `Form` API
and looks very much like a bug.

My proposed solution does not introduce any breaking changes and can even
be easily reverted to its original behavior with the patch I also proposed
for #35542.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:2>

Django

unread,
Jul 3, 2024, 6:00:32 AM7/3/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: new
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Christophe Henry):

* resolution: needsinfo =>
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:3>

Django

unread,
Jul 5, 2024, 1:13:15 PM7/5/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution:
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* resolution: => worksforme
* status: new => closed

Comment:

Replying to [comment:2 Christophe Henry]:

Hello Christophe, thank you for the extra details. In this sentence below:

> But if you do this, you won't get the result you expect when rendering
form with `{{ form }}`

I would need you to describe in detail what is the expected result from
your view, because what I would usually expect is what I get when doing a
local test.
For reference, this is my form and view, inspired by your code:

{{{#!python
from django import forms
from django.contrib import messages
from django.core.exceptions import ValidationError
from django.http import HttpResponseRedirect
from django.shortcuts import render


class FooForm(forms.Form):
foo_select = forms.CharField()

def __init__(self, user, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields["foo_select"].help_text = f"Indicate
{user.first_name}'s pet name"

def clean_foo_select(self):
value = self.cleaned_data.get("foo_select")
if value != "fido":
raise ValidationError("foo_select should be fido")


def foo_view(request):
form = FooForm(user=request.user)
if request.method == "POST":
form = FooForm(user=request.user, data=request.POST)
if form.is_valid():
messages.success(request, f"Valid {form.cleaned_data=}! :-)")
return HttpResponseRedirect(".")

messages.error(request, f"INVALID {form.data=}! :-(")

return render(request, "foo.html", {"form": form})
}}}

And in my template, the correct help text is shown at all times (on GET,
on form submission with errors and on form submission without errors).

Because of the above, I'll close as `worksforme`, but please fell free to
comment with further details. Ideally you would provide a minimal sample
Django project showcasing the issue and describing what you expect to get.

Thanks again!
Natalia.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:4>

Django

unread,
Jul 8, 2024, 4:23:11 AM7/8/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution:
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Christophe Henry):

Replying to [comment:4 Natalia Bidart]:

> And in my template, the correct help text is shown at all times

That's because when you do {{{ self.fields["foo_select"].help_text =
f"Indicate {user.first_name}'s pet name" }}}, at this stage,
[https://github.com/django/django/blob/main/django/forms/forms.py#L104
self._bound_fields_cache] may not be populated. But if you do the
following:

{{{#!python
class FooForm(forms.Form):
foo_select = forms.CharField()
... # Other fields

def __init__(self, user, *args, **kwargs):
super().__init__(*args, **kwargs)
# candid dev not knowing the difference between self[] and
self.fields[]
# setting avariable for every field
for field in self:
field.should_do_something = False
self.fields["foo_select"].help_text = f"Indicate
{user.first_name}'s pet name"

# demonstration
assert self["foo_select"].help_text ==
self.fields["foo_select"].help_text
}}}

Please don't tell me this is not a valid situation. This happened to me on
a project recently and I got a hard time tracking down why this happened.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:5>

Django

unread,
Jul 8, 2024, 4:23:17 AM7/8/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: new
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Christophe Henry):

* resolution: worksforme =>
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:6>

Django

unread,
Jul 8, 2024, 4:30:08 AM7/8/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: new
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Christophe Henry):

BTW, I think {{{ BoundField.html_name }}}, {{{
BoundField.html_initial_name }}} and {{{ BoundField.html_initial_id }}}
should also be properties since they are purely computed variables from
{{{ Form }}}.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:7>

Django

unread,
Jul 8, 2024, 9:46:45 AM7/8/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution:
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* cc: David Smith (added)
* resolution: => worksforme
* status: new => closed

Comment:

Hey Christophe, thank you for the extra details. For what it's worth, I'm
not saying that your issue is invalid; I fully acknowledge that it caused
an error in your project and that you've invested time in finding the root
cause.

However, there's another aspect to triaging a bug in Django, which
involves determining whether a report pertains to a niche use case or
something that would affect the majority of users. Django is a framework
designed to provide robust and reliable solutions for common scenarios.
It's built on a mature code base where code changes should be carefully
evaluated.

To accept a ticket like this, we need two things:

1. A Django minimal project showcasing the issue. I added the `assert`
suggested above and it's not failing for me. My code is exactly the one
posted above with the adding of `assert self["foo_select"].help_text ==
self.fields["foo_select"].help_text`.

2. To fully describe and understand the use case for your code. Even if
the `assert` would fail (which as I mentioned is not failing for me), it's
unclear if this is actually a valid Django logic pattern. Could you please
describe in words what the goal is in accessing `self[<field name>]` as
you propose? It seems unusual to access a BoundField inside the Form's
`__init__` (since the form hasn't been fully created yet). See
[https://docs.djangoproject.com/en/5.0/ref/forms/api/#accessing-the-
fields-from-the-form the docs] for ''Accessing the fields from the form''.

I know we've mentioned this in other tickets you've created, but it's
important to reiterate: Django is a community-driven project. While the
Fellows oversee maintenance and ensure timely releases, most design
decisions and feature changes/additions are made through community
consensus. The forum is the primary place to seek that consensus, as it
notifies the most interested parties (unlike here). Since I can't
reproduce the issue with the provided details and this requires broader
discussion, the best course of action to post in the
[https://forum.djangoproject.com/ Django Forum] presenting the use case
for wider feedback.

Thanks again!
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:8>

Django

unread,
Jul 9, 2024, 3:30:00 AM7/9/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution:
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Christophe Henry):

I'm sorry but I'm tired of this back and forth, both here #35542. Merging
such basic code should not be that much of a burden, espacially when I
took extra care to write the tests and update the doc so that the PR is
directly mergeable. In both thos tickets, I was advise to take it to the
forum to achieve community consensus but what am I supposed to do when the
threads I open don't get attention at all and that even debating to try to
achive consensus is impossible, while in the meantime, I need bugfixes or
features merged?

There's definitely something wrong in the development process of Django
and that won't help attracting new contributors.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:9>

Django

unread,
Jul 10, 2024, 10:21:24 AM7/10/24
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution:
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia Bidart):

Hello Christophe,

I understand your frustration, and I acknowledge the effort you've put
into writing tests and updating documentation for your PRs. It's clear
you've encountered challenges in achieving consensus on certain issues. As
mentioned, Django is a community-driven project, so contributions and
engagement vary based on availability and interest. To generate more
engagement in proposals and discussions:

* Clearly present the use case or need for change with a practical
example.
* Evaluate multiple solutions rather than focusing on a single
implementation.
* Seek input on potential framework-provided solutions that may not be
widely known.

Without these foundational steps, gaining attention and consensus can be
challenging. Given Django's maturity and widespread adoption, each
proposed change requires thorough evaluation and justification.

Specifically about this ticket, your proposed change in
[https://github.com/django/django/pull/18289 PR #18289] introduces a
breaking change, necessitating a clear deprecation plan and strong
rationale to justify it (think of those child classes that use `label` or
`help_text` as instance variables, and how assignments to these would
break when converted to properties). Furthermore, it's important to note
that you have not yet provided a reproducible case or detailed use case
explanation. These are essential for initiating meaningful discussions and
gaining broader community support.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:10>
Reply all
Reply to author
Forward
0 new messages