formsets should be iterable

[kenth]

I have a inlineformset where I need to be able to display some of the "extra" forms (that I initialize) before some of the "initial" forms retrieved from the db. This would be easy if I could just override formset's iter method and get the forms to come out in an order different from the creation order. This patch adds this hook.

This patch also makes it possible to say "for form in formset" (instead of "for form in formset.forms") in both code & templates. There are no backward compatibility issues.

The pattern "for form in formset" is also closer to the "for field in form" idiom.

[kenth]

Another use of __iter__ in formsets is to allow a formset panel of questions to be presented in random order. Override __iter__ to assign a random number to each form, and then display in sorted order.

[pandres]

Your path seems to add another way to do something that is already easy to do (and it's done all around in the code). It goes against the principle of "there should be one obvious way to do it"

[Russell Keith-Magee]

Although Pandres' comment is correct (i.e., there is already a way to iterate over forms), I think what you're proposing makes sense, both in terms of consistency in the API, and in terms of providing a point at which behavior can be modified.

[kenth]

Added tests. As a result, I added getitem & len methods to the patch. Several of the tests indexed formset.forms.

Passes tests. Ready for review.

[Erin Kelly]

Instead of being a generator function, the __iter__ method should simply return iter(self.forms). Why add a layer of wrapping when you can delegate?

Why are the __getitem__ and __len__ methods necessary? Since they require iterating over the full formset, they are less efficient than the O(1) methods on the underlying list, which bugs me. Why not let them also delegate to self.forms, and anybody who needs to customize __iter__ should just customize those as well?

[kenth]

Improved patch delegating iter to self.forms

[kenth]

@ikelly, thanks for your help. Duh on returning iter(self.forms). I too don't like the __getitem__ and __len__ methods, but they're required to pass the unit tests (mainly in modelforms). And since they're required, I implemented ones that worked correctly -- if slowly -- instead of requiring all three methods be overridden if __iter__ is. My formset which required reordering rendered forms (e.g. a sparse many-to-many relationship table of per-user ratings of beatles albums that I want to be rendered in album order, mixing "existing" and "extra" forms) would only require the __iter__ method. An inefficient but correct method for the (probably never used except in unit test) ancillary methods seemed like the way to go.

Of course, you improved my base __iter__ method. Any suggestions on the others would be great. I'd like to get the __iter__ hook into django. Python is not (yet) my best language.

[anonymous]

Maybe not really needed but shouldn't the original way also be tested?
The patches change everything to this new method which is fine but maybe leave tests for the old way of doing it to make sure existing code doesn't get broken?

[kenth]

The "original way" was to iterate over self.forms. The new way (iterating over self) also iterates over self.forms -- but via delegate per Ian's comments above. The only way to break the old way that I can think of would be to rename self.forms. I guess it could be tested for, I just didn't think it was needed. Is it?

[kenth]

Also, many contrib modules continue to iterate over self.forms (like contrib.admin). I changed only the "core".

[Russell Keith-Magee]

(In [14986]) Fixed #14655 -- Made formsets iterable. This allows a slightly more natural iteration API (for form in formsets), and allows you to easily override the form rendering order. Thanks to Kent Hauser for the suggestion and patch.

[Russell Keith-Magee]

I've just committed this, but a couple of comments for future reference:

• We prefer patches to be submitted as a a single patch, not multiple partial patches. I was about to bounce this ticket due to a lack of documentation until I realized that documentation was contained in an different patch ticket.
• You don't *ever* change an existing test case. Existing unittests are regressions for existing behavior. The fact that you're introducing a new behavior doesn't invalidate the existing behavior.
• Code still needs to run under Python 2.4; this means you can't use context managers.