Opened 3 months ago

Closed 3 months ago

#35723 closed Cleanup/optimization (invalid)

Change django.forms.formsets.all_valid() to use generator instead of list comprehension

Reported by: Prashant Rana Owned by:
Component: Forms Version: dev
Severity: Normal Keywords: formsets, validation
Cc: Prashant Rana Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

in function all_valid in formsets.py https://github.com/django/django/blob/main/django/forms/formsets.py#L576
we are using list comprehension to check if each individual formset is valid or not and then adding that to list and then computing all. in this approach, we have to go through all the formsets even if one of them is not valid and then check if all of them are true.

with this approach, we have to check all formsets first and then check if all of list is valid ie True. An extra computation is happening even after if encounter not valid and in memory is also used .

my purposal is to use generator comprehension here instead of list comprehension.
benefits include, no need to validate all formset if one of formset is invalid and less in memory usages.

so code should look like

from

def all_valid(formsets):
    """Validate every formset and return True if all are valid."""
    # List comprehension ensures is_valid() is called for all formsets.
    return all([formset.is_valid() for formset in formsets])

to

def all_valid(formsets):
    """Validate every formset and return True if all are valid."""
    return all(formset.is_valid() for formset in formsets)

Change History (1)

comment:1 by Tim Graham, 3 months ago

Resolution: invalid
Status: newclosed
Summary: formset all_valid use generator comprehension instead of list comprehensionChange django.forms.formsets.all_valid() to use generator instead of list comprehension

That would make the behavior not match the docstring ("Validate every formset") and it causes a test failure:

======================================================================
FAIL: test_invalid (forms_tests.tests.test_formsets.AllValidTests.test_invalid)
all_valid() validates all forms, even when some are invalid.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/tests/forms_tests/tests/test_formsets.py", line 1962, in test_invalid
    self.assertEqual(formset2._errors, expected_errors)
AssertionError: None != [{'votes': ['This field is required.']}, {'votes': ['This field is required.']}]
Note: See TracTickets for help on using tickets.
Back to Top