On Mon, 2007-05-21 at 23:14 -0500, Gary Wilson wrote: > Malcolm Tredinnick wrote: > >> #3718 - newforms.Form.clean should have access to field errors [1] > > > > I'm not sure what this really gains. Fixing the second part of #3896 > > means that all the necessary information is in cleaned_data. I'm -0 on > > this, because I feel it's pretty pointless, merely providing more than > > one way to do something (not a good thing). > > > > Now convince me I'm wrong. What use-case have I overlooked? > > I would say that checking the errors property should be the one way to > see if there have been errors, not checking that every field in my model > has a key in cleaned_data. Without access to errors, you're going to > have one hell of an if statement with even a small form.
Only if you code it badly. A for loop over self.cleaned_data works. > Add in > non-required fields and things get ugly even faster since it's not an > error if a non-required field has no key in cleaned_data. *shrug* In most cases you're going to have to access self.fields in any case. I'm still -0. But that really is -0 -- I have a slight preference, but I'll manage to keep on living whichever way it goes; quite happily, too. :-) > > Add in a subclass and now you've got the subclass having to know the > fields of the base class(es). Either that or a for loop looping through > self.base_fields to check that every field is in cleaned_data. Again, > getting even uglier with non-required fields. > > >> #3896 - pass value to field specific clean function [2] > > > > Grr...don't put two issues in one ticket! > > > > I'm -1 on the first part because it's an unnecessary backwards > > incompatibility change for the most part. It's not like it's a massive > > performance improvement in an area that is currently slow. > > > > Definitely +1 on the second part, though. Removing the first assignment > > to self.cleaned_data would make a lot of sense. > > I could have created a new ticket for the "self.cleaned_data retaining > the value even if clean_XXX() fails" problem, but by taking out the > first assignment to self.cleaned_data you *have to* pass the field's > value to clean_XXX() since the value would no longer be in > self.cleaned_data for clean_XXX() to access it. I figured opening a new > ticket would not be worthwhile, I could have been wrong. I'm sort of convinced. I'd forgotten that you would access the value through cleaned_data. We could still fix it in a backwards compatible fashion: if clean_<fieldname> raises a ValidationError remove the value from cleaned_data, but maybe it's not worth it. I'm 0 on the first part and +1 on the second, then (since we can fix the second without the first). Malcolm --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
