Opened 17 years ago

Closed 17 years ago

Last modified 17 years ago

#3376 closed (wontfix)

newforms.Form.clean_data is not read-only

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

Description (last modified by Adrian Holovaty)

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 Holovaty 17 years ago.
Implementation of immutable clean_dict

Download all attachments as: .zip

Change History (8)

comment:1 by anonymous, 17 years ago

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 by Adrian Holovaty, 17 years ago

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 by Adrian Holovaty, 17 years ago

Triage Stage: UnreviewedDesign decision needed

comment:4 by anonymous, 17 years ago

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 by Adrian Holovaty, 17 years ago

Description: modified (diff)

(Fixed formatting in description.)

comment:6 by Adrian Holovaty, 17 years ago

Resolution: wontfix
Status: newclosed

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.

by Adrian Holovaty, 17 years ago

Implementation of immutable clean_dict

comment:7 by Adrian Holovaty, 17 years ago

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

Note: See TracTickets for help on using tickets.
Back to Top