Code

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#3376 closed (wontfix)

newforms.Form.clean_data is not read-only

Reported by: jfindlay@… Owned by: adrian
Component: Forms Version: master
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by adrian)

Is it possible to make newforms.Form.clean_data immutable? I'm kind of surprised it isn't, but I don't know the meaning or purpose of all things.

$ python manage.py shell
Python 2.4.4c1 (#2, Oct 11 2006, 21:51:02) 
[GCC 4.1.2 20060928 (prerelease) (Ubuntu 4.1.1-13ubuntu5)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django import newforms
>>> class form(newforms.Form):
...   a = newforms.IntegerField()
...   b = newforms.CharField()
... 
>>> d = {'a': 1, 'b': 'field b'}
>>> f = form(d)
>>> f.is_valid()
True
>>> f.clean_data
{'a': 1, 'b': u'field b'}
>>> f.clean_data['a'] = 2
>>> f.clean_data
{'a': 2, 'b': u'field b'}

Attachments (1)

3376_immutable_clean_dict.diff (4.8 KB) - added by adrian 7 years ago.
Implementation of immutable clean_dict

Download all attachments as: .zip

Change History (8)

comment:1 Changed 7 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Here's my example again, unwikified, or wikified depending on how you think.

$ python manage.py shell
Python 2.4.4c1 (#2, Oct 11 2006, 21:51:02) 
[GCC 4.1.2 20060928 (prerelease) (Ubuntu 4.1.1-13ubuntu5)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django import newforms
>>> class form(newforms.Form):
...   a = newforms.IntegerField()
...   b = newforms.CharField()
... 
>>> d = {'a': 1, 'b': 'field b'}
>>> f = form(d)
>>> f.is_valid()
True
>>> f.clean_data
{'a': 1, 'b': u'field b'}
>>> f.clean_data['a'] = 2
>>> f.clean_data
{'a': 2, 'b': u'field b'}
>>> 

comment:2 Changed 7 years ago by adrian

Hmm, I could go either way on this. Making clean_data an immutable dictionary would introduce a tiny bit of overhead, as we'd have to define our own ImmutableDict class. Do you really think having clean_data be mutable is a huge problem?

comment:3 Changed 7 years ago by adrian

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 7 years ago by anonymous

I supposed that validated form data would have been treated in the same read-only manner as request.GET and request.POST and also considering the following paragraph from http://www.djangoproject.com/documentation/newforms/.

If you have a bound Form instance and want to change the data somehow, or if you want to bind an unbound Form instance to some data, create another Form instance. There is no way to change data in a Form instance. Once a Form instance has been created, you should consider its data immutable, whether it has data or not.

It seemed contradictory to me to have form data be considered immutable but then be able to modify it because the clean_data dict is not read-only.

comment:5 Changed 7 years ago by adrian

  • Description modified (diff)

(Fixed formatting in description.)

comment:6 Changed 7 years ago by adrian

  • Resolution set to wontfix
  • Status changed from new to closed

I implemented this in my local Django checkout but changed my mind at the last minute -- I don't see a good reason to make clean_data immutable, and I think it might be more of an annoyance than a protection. (And what would it protect from, anyway?)

For posterity, I've uploaded a patch of the changes I made, in case we change our minds in the future.

Changed 7 years ago by adrian

Implementation of immutable clean_dict

comment:7 Changed 7 years ago by adrian

Note that the 3376_immutable_clean_dict.diff has a small problem -- it mistakenly removes the render_value unit tests.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.