#22786 closed Cleanup/optimization (fixed)
Document that value_from_datadict may be called more than once
| Reported by: | Daniel Hahler | Owned by: | nobody |
|---|---|---|---|
| Component: | Documentation | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I've noticed that a widget's value_from_datadict gets called twice when changing a model in the admin, which could and
should probably be avoided to save extra processing, especially since the second call is only for construct_change_message.
While there might be use cases for this (which?), it seems like value_from_datadict is not meant to return a different result on the second call anyway?!
If this is the desired behavior, it should be documented, and a recommendation being added to cache any extra processing (like downloading external files).
…/django-1.7.x/django/forms/forms.py(353)full_clean()
351 return
352
--> 353 self._clean_fields()
354 self._clean_form()
355 self._post_clean()
…/django-1.7.x/django/forms/forms.py(362)_clean_fields()
360 # Each widget type knows how to retrieve its own data, because some
361 # widgets split data over several HTML fields.
--> 362 value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name))
363 try:
364 if isinstance(field, FileField):
> …/project/dh_utils/forms.py(123)value_from_datadict()
121 def value_from_datadict(self, data, files, name):
122 import ipdb; ipdb.set_trace()
--> 123 return None
124 upload = super(ClearableURLFileInput, self).value_from_datadict(data, files, name)
125 if not self.is_required and CheckboxInput().value_from_datadict(
…/django-1.7.x/django/contrib/admin/options.py(1369)changeform_view()
1367 return self.response_add(request, new_object)
1368 else:
-> 1369 change_message = self.construct_change_message(request, form, formsets)
1370 self.log_change(request, new_object, change_message)
1371 return self.response_change(request, new_object)
…/django-1.7.x/django/contrib/admin/options.py(951)construct_change_message()
949 """
950 change_message = []
--> 951 if form.changed_data:
952 change_message.append(_('Changed %s.') % get_text_list(form.changed_data, _('and')))
953
…/django-1.7.x/django/forms/forms.py(419)changed_data()
417 for name, field in self.fields.items():
418 prefixed_name = self.add_prefix(name)
--> 419 data_value = field.widget.value_from_datadict(self.data, self.files, prefixed_name)
420 if not field.show_hidden_initial:
421 initial_value = self.initial.get(name, field.initial)
> …/project/dh_utils/forms.py(123)value_from_datadict()
121 def value_from_datadict(self, data, files, name):
122 import ipdb; ipdb.set_trace()
--> 123 return None
124 upload = super(ClearableURLFileInput, self).value_from_datadict(data, files, name)
125 if not self.is_required and CheckboxInput().value_from_datadict(
Attachments (1)
Change History (6)
comment:1 by , 11 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 11 years ago
| Has patch: | set |
|---|
Most of the time, value_from_datadict is just retrieving some content from a dictionary with light processing, so I don't think it would justify caching its result. Attached a documentation proposal.
comment:3 by , 11 years ago
| Component: | contrib.admin → Documentation |
|---|---|
| Summary: | value_from_datadict gets called twice (additionally in changeform_view) → Document that value_from_datadict may be called more than once |
| Triage Stage: | Accepted → Ready for checkin |
Might replace "costly procedures" with "expensive processing" which appears elsewhere in the docs.
comment:4 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Haven't looked into whether the second call can be avoided, but accepting for documentation if not.