I haven't been following everything, but I do have a couple comments
to make here.
On Fri, Jan 23, 2009 at 8:04 AM, mrts <m...@mrts.pri.ee> wrote:
> works both for forms and models with the proposed approach (all_values
> is model_instance.__dict__.copy() in model field validation case).
One thing to consider is that not all data is always available via the
dictionary, particularly with regard to models. Anything that uses a
descriptor (such as relations to other models) only stores a bare
minimum of information in the instance dict. The rest is retrieved
when accessed *as an attribute*, a fact which is entirely lost in your
example. I understand you're trying not to bundle it too much with the
way models work, so maybe this was intentional, but I do think it's
worth noting, just in case it's not intentional.
> Handling this specially for model instances would be really ugly:
>
> def __call__(self, value, all_values={}, instance=None):
> if instance is not None:
> if hasattr(self.other_field, instance) and getattr
> (self.other_field, instance) == self.other_value and not value:
> raise ValidationError(self.error_message, REQUIRED)
> elif self.other_field in all_values and all_values
> [self.other_field] == self.other_value and not value:
> raise ValidationError(self.error_message, REQUIRED)
>
> and entirely unnecessary under the all_values approach.
I agree that it's a bit ugly, but I'd hardly call it unnecessary,
given the possibly of using descriptors on models. Forms aren't
susceptible to the same problem, because their values are stored in a
true dict regardless. Offhand, I can only think of one way to get a
uniform syntax that works the way you seem to want it to, and that's
by bundling forms and models in some other objects with a unified
interface, designed solely for validation. Something like the
following (untested, coding off the top of my head):
class ValidatableModel(object):
def __init__(self, instance):
self._instance = instance
def __getitem__(self, name):
if hasattr(self._instance, name):
return getattr(self._instance, name)
raise KeyError(name)
class ValidatableForm(object):
def __init__(self, instance):
self._instance = instance
def __getitem__(self, name):
return self._instance.cleaned_data[name]
There could be more methods to be able to iterate over fields and
whatever other niceties you would want to include, but that way you
have a uniform dict interface to work with, regardless of which object
type gets passed in, and it still preserves the ability to use
descriptors on model attributes.
That said, I'm not particularly fond of that solution myself, because
I personally feel that models and forms *should* be treated a bit
differently; the field types for each are just too different in their
capabilities. I haven't been involved with this validation stuff
except for improving file support for it, but I'm not sure a unified
approach would be as big of a win as you claim.
-Gul
(and if I'm missing some obvious bit of history here, feel free to correct me)
--~--~---------~--~----~------------~-------~--~----~
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
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
