Opened 3 days ago

Last modified 11 hours ago

#35843 assigned Cleanup/optimization

Changing the rendering order of formsets is not clear

Reported by: Matthew Pava Owned by: Clifford Gama
Component: Forms Version: 5.1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a formset that I'd like to modify the order of the forms in. I did overwrite the forms cached_property with a simple sorted addition to the base forms cached_property, but I soon discovered that data wasn't saving properly in some cases.

Apparently, I went about it all wrong. There's this little tiny tidbit here about rendering forms:
https://docs.djangoproject.com/en/5.1/topics/forms/formsets/#:~:text=Iterating%20over%20a%20formset%20will,which%20returns%20the%20corresponding%20form

Iterating over a formset will render the forms in the order they were created. You can change this order by providing an alternate implementation for the __iter__() method.

Formsets can also be indexed into, which returns the corresponding form. If you override __iter__, you will need to also override __getitem__ to have matching behavior.

Okay, but I've never implemented an __iter__ method, and based on what I've read, you also have to implement a __next__ method. And you explicitly tell me to implement the __getitem__ method. I understand if you don't want to make changing the rendering ordering easier to implement in Django; however, I would appreciate more and better documentation including an example of how exactly to implement any of these methods to change the rendering order of a formset.

Change History (7)

comment:1 by Matthew Pava, 3 days ago

I was able to get it to work by basically putting the sorted expression into the __iter__ method and removing my implementation of forms.

    def __iter__(self):
        return (i for i in sorted([f for f in self.forms], key=self._sort_form_key, reverse=True))

comment:2 by Sarah Boyce, 3 days ago

Component: DocumentationForms
Triage Stage: UnreviewedAccepted

I agree an example would be nice

comment:3 by Clifford Gama, 2 days ago

I'm not sure that we need to include an example, since the iterator protocol is Python and not Django, and is documented in python docs here, and emulating container types is documented here. Considering that, maybe a link to the docs or to an excellent tutorial may suffice, or we risk straying out of the scope Django docs and into into core Python fundamentals.

Version 0, edited 2 days ago by Clifford Gama (next)

comment:4 by Sarah Boyce, 2 days ago

I agree with you Clifford, but I was also quite tempted that we remove those two sentences entirely and reduce it just to:
Iterating over a formset will render the forms in the order they were created.

I'm not sure how much value we add by stating you can update the order of iterating by overriding the iterator (this should be the case for most things).

comment:5 by Clifford Gama, 2 days ago

I think we can keep the sentences. This ticket shows that someone needs to reorder formsets, and pointing them to what they'd need to do to accomplish that is helpful. Maybe we can change the sentences so that we don't mention the specific methods__iter__, __get_item__ and __next__: something like: "you can change the order by overriding the iterator protocol" with a link the relevant Python docs. Or maybe keep the methods, but make them link to their own docs.

comment:6 by Natalia Bidart, 2 days ago

I also think that keeping the sentence helps since I wouldn't have considered overriding __iter__ to control the formset ordering. I agree with Clifford that we should say something along the line "this is really implementing the methods that define an object as an iterable". For example pointing to https://docs.python.org/3/glossary.html#term-iterable, but please note that formsets provide an iterable API (which is slightly different from an iterator API: the former implement __iter__ and __getitem__, and the latter __iter__ and __next__).

comment:7 by Clifford Gama, 11 hours ago

Has patch: set
Owner: set to Clifford Gama
Status: newassigned
Note: See TracTickets for help on using tickets.
Back to Top