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. 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. 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. >> #4271 - Form should use a copy of data passed to it [3] > > Make up your mind! You want to save a single variable access in #3896, > but do a whole dictionary copy every time here??! :-) > > I'm a bit torn about this, because QueryDict.copy() isn't free and the > use-case you mention in the bug isn't particularly common and isn't > currently impossibly to do at the moment, by doing the copy() in the > Form constructor. > > I'm probably -0 for that reason. Not something I feel particularly > strongly about, though. > >> #4297 - make BaseForm's __errors attribute non-private [4] > > Whoops. I thought I'd already changed this one to Accepted. Apparently > I'd only got as far as making the decision in my head. I agree with you > on this. I couldn't think of a reason to hide that value (and making it > "_errors", rather than "errors" is the right change). > > I worry about people who want to hide away error messages after the > fact, rather than fixing the problems in the right place (whatever > validation generated the errors), but that's their problem. We'll > provided shotguns and bullets if it's not too hard, they can provide the > feet and entertain themselves at will. It's not all about hiding error messages though. If I had a Form subclass that had a full_clean() that called its parent and then did some other things and created errors, now the __errors dict would be split into two mangled instance attributes (one for each class). Not fun. I like my forms fully loaded, thank you. Gary --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
