Form.default_renderer is ignored in formsets.

[Ryan Burt]

Overriding default_renderer functionality in the ModelForm class

This issue only came into effect on my project upon upgrading from 3.2 to 4.2

settings.py:

FORM_RENDERER = 'django.forms.renderers.TemplatesSetting'

myapp/forms.py:

class MyModelForm(forms.ModelForm):
    default_renderer = CustomRenderer
   
    class Meta:
        model = MyModel

Each time the forms api hits the forms.forms.BaseForm.init() method in forms/forms.py, renderer is always not null and the condition for using the default_renderer is never reached:

(line 120 in forms/forms.py)
# Initialize form renderer. Use a global default if not specified
# either as an argument or as self.default_renderer.
if renderer is None:
    if self.default_renderer is None:
        renderer = get_default_renderer()
    else:
        renderer = self.default_renderer
        if isinstance(self.default_renderer, type):
             renderer = renderer()
    self.renderer = renderer

Different FORM_RENDERER values seem to have no effect. If I override the ModelForm init method the renderer is passed in the kwargs, so it is possible to set it to None. I'm interested in understanding if I have a compatibility issue in my settings as a result of upgrading versions, or if the above code block could be restructured to preference default_renderer if it is not none:

# Initialize form renderer. Use a global default if not specified
# either as an argument or as self.default_renderer.
if self.default_renderer:
    if isinstance(self.default_renderer, type):
        renderer = self.default_renderer()
    else:
        renderer = self.default_renderer
elif renderer is None:
        renderer = get_default_renderer()
self.renderer = renderer 

[Mariusz Felisiak]

Can you provide a sample project that reproduces the issue? How do you initialize forms? Please reopen the ticket if you can provide more details.

or if the above code block could be restructured to preference default_renderer if it is not none

Unfortunately not, the renderer explicitly passed in the arguments should always be preferred as default_renderer is a fallback.

[Ryan Burt]

I'll provide a sample project shortly, but to answer the second question, I'm initializing my forms through the modelformset_factory.

This got me on the right track, I've reviewed the code in forms/formsets.py and the renderer is passed in the attrs via 'render': renderer or get_default_renderer() on line 571.
This an addition from commit ​456466d.

If I suppress FORM_RENDERER in my settings.py, get_default_renderer() returns the default of 'django.forms.renderers.DjangoTemplates', so this could be an unintended consequence of the changes in that commit

[Mariusz Felisiak]

Good catch. We should still pass BaseFormSet.renderer when instantiating forms and it's set manually, it can be tricky to distinguish these cases.

[David Smith]

Here's a scenario.

class CustomFormRenderer(TemplatesSetting):
    ...

class MyForm(Form):
    name = CharField()
    default_renderer = CustomFormRenderer

class CustomFormsetRenderer(Jinja2):
    ...

MyFormSet = formset_factory(MyForm, renderer=CustomFormsetRenderer)

Should MyForm be rendered with the CustomFormRenderer of the CustomFormsetRenderer.

The way that I've thought about this to date is that whatever gets used at the highest level (formset -> form -> widget) gets passed down to the lower levels so that the same renderer is used for the whole form/formset.

Does passing your custom renderer modelformset_factory work for you?

I think even if no code changes were agreed for this ticket there may be some doc changes needed.

[Ryan Burt]

Hi,

Passing the renderer to modelformset_factory does indeed work, though it needs to be instantiated object rather than passing the class.
I think the concern is that in your scenario, setting default_renderer = CustomFormRenderer is still unsupported as the code block that uses the default_renderer is inaccessible.
This leaves a confusing mix of behaviour for the ModelForm class as the default_renderer will work when instantiating directly, but not when using formset_factory . I agree with the hierarchical intent that lower level methods shouldn't override supplied variables from their parents.

I think that altering the attrs in formset_factory from:

attrs = {
        "form": form,
        "extra": extra,
        "can_order": can_order,
        "can_delete": can_delete,
        "can_delete_extra": can_delete_extra,
        "min_num": min_num,
        "max_num": max_num,
        "absolute_max": absolute_max,
        "validate_min": validate_min,
        "validate_max": validate_max,
        "renderer": renderer or get_default_renderer(),
    }

To:

attrs = {
        "form": form,
        "extra": extra,
        "can_order": can_order,
        "can_delete": can_delete,
        "can_delete_extra": can_delete_extra,
        "min_num": min_num,
        "max_num": max_num,
        "absolute_max": absolute_max,
        "validate_min": validate_min,
        "validate_max": validate_max,
        "renderer": renderer,
    }

Is a non breaking change, as the return is the very next line, which instantiates the Form class:

return type(form.__name__ + "FormSet", (formset,), attrs)

And the renderer variable is not used anywhere prior inside BaseForm.__init__ :

    def __init__(
        self,
        data=None,
        files=None,
        auto_id="id_%s",
        prefix=None,
        initial=None,
        error_class=ErrorList,
        label_suffix=None,
        empty_permitted=False,
        field_order=None,
        use_required_attribute=None,
        renderer=None,
    ):
        self.is_bound = data is not None or files is not None
        self.data = MultiValueDict() if data is None else data
        self.files = MultiValueDict() if files is None else files
        self.auto_id = auto_id
        if prefix is not None:
            self.prefix = prefix
        self.initial = initial or {}
        self.error_class = error_class
        # Translators: This is the default suffix added to form field labels
        self.label_suffix = label_suffix if label_suffix is not None else _(":")
        self.empty_permitted = empty_permitted
        self._errors = None  # Stores the errors after clean() has been called.

        # The base_fields class attribute is the *class-wide* definition of
        # fields. Because a particular *instance* of the class might want to
        # alter self.fields, we create self.fields here by copying base_fields.
        # Instances should always modify self.fields; they should not modify
        # self.base_fields.
        self.fields = copy.deepcopy(self.base_fields)
        self._bound_fields_cache = {}
        self.order_fields(self.field_order if field_order is None else field_order)

        if use_required_attribute is not None:
            self.use_required_attribute = use_required_attribute

        if self.empty_permitted and self.use_required_attribute:
            raise ValueError(
                "The empty_permitted and use_required_attribute arguments may "
                "not both be True."
            )

        # Initialize form renderer. Use a global default if not specified
        # either as an argument or as self.default_renderer.
        if renderer is None:
            if self.default_renderer is None:
                renderer = get_default_renderer()
            else:
                renderer = self.default_renderer
                if isinstance(self.default_renderer, type):
                    renderer = renderer()
        self.renderer = renderer

Essentially, if renderer is none when calling formset_factory , get_default_renderer() is still called lower in the api if required, but still before self.renderer is ever utilised in the form widgets. This improves your scenario as you could do something like:

class CustomFormRendererPrimary(TemplatesSetting):
    ...

class CustomFormRendererSecondary(DjangoTemplates):
    ...

class MyForm(Form):
    name = CharField()
    default_renderer = CustomFormRendererPrimary

class CustomFormsetRenderer(Jinja2):
    ...
if use_secondary:
    MyFormSet = formset_factory(MyForm, renderer=CustomFormRendererSecondary())
else:
    MyFormSet = formset_factory(MyForm)

Adding the caveat to the docs that default_renderer through formset_factory is unsupported and that passing the renderer directly is the supported method could suffice, as I'm not sure how many projects are affected by this and haven't figured out to pass renderer directly (or override __init__ to set the renderer like I did originally).

[Christopher Cave-Ayland]

I've been looking at this as part of the sprints for DjangoCon EU. I've put together ​a test (gist pasted below) that I believe captures the desired behaviour discussed above.

    def test_form_default_renderer(self):
        """
        In the absense of a renderer passed to formset_factory, the default_renderer
        attribute of the Form class should be respected.
        """
        from django.forms.renderers import Jinja2

        class ChoiceWithDefaultRenderer(Choice):
            default_renderer = Jinja2

        data = {
            "choices-TOTAL_FORMS": "1",
            "choices-INITIAL_FORMS": "0",
            "choices-MIN_NUM_FORMS": "0",
        }

        ChoiceFormSet = formset_factory(ChoiceWithDefaultRenderer, renderer=None)
        formset = ChoiceFormSet(data, auto_id=False, prefix="choices")
        self.assertIsInstance(
            formset.forms[0].renderer, ChoiceWithDefaultRenderer.default_renderer
        )

This test currently fails but can be made to pass via the above change suggested via @RyanBurt.

The change unfortunately is breaking however and causes 14 other test failures. The root cause seems to be that the renderer attribute for the resulting formset class ends up as None even though it is expected not to be.

To progress this ticket I think some more sophisticated changes to BaseFormSet are required to determine the appropriate renderer to use for forms. I'll aim to update after some further experimentation.,

[Kees Hink]

My and my colleagues tried to reproduce and illustrate the issue. We did so in this repository: ​https://github.com/fourdigits/ticket_34532/

Based on this, we feel that the issue as reported by Ryan Burt has merit: when we use formset_factory to create a formset, the default should be to use the form's renderer.

Changing this behavior, as Christopher Cave-Ayland already tried, would be welcome.

[Christopher Cave-Ayland]

​https://github.com/django/django/pull/16916

[Mariusz Felisiak]

Marking "Needs" flags based on David's ​comment.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 95e4d6b:

Fixed #34532 -- Made formset_factory() respect Form's default_renderer.

Co-authored-by: David Smith <smithdc@…>