django.contrib.gis.forms.fields.GeometryField completely ignores the
required attribute in its clean() method. Instead it checks if the
model field is nullable, which needlessly ties forms and model code. I
realized this because I have a model with a non-nullable GeometryField
but need a non-required GeometryField in my form, because if it's null
I set it inside clean().

I wanted to confirm if this should be considered a bug before opening
a ticket.

My proposed solution would be to call Field's clean() instead of
checking self.null.

On Mar 25, 12:20 pm, Fidel Ramos <fidelra...@gmail.com> wrote:

Isn't the checking of `self.null` that you've identified as the
problem already in the `GeometryField.clean` method?  Are you
proposing a recursive call?

I agree that GeometryField is ripe to be improved -- but I'm unclear
on exactly what is the problem and the proposed solution here.

-Justin

On 27 mar, 19:49, Justin Bronn <jbr...@gmail.com> wrote:

Thanks for answering Justin.

I'll show the problem with a simple example. Suppose the following
form:

class MyForm(forms.Form):
    point = GeometryField(required=False)

Do you think the point field can be left blank and then the form
validates? Because "required" is an attribute used by the Field
superclass, one should assume that the point field could be left
blank, but because of GeometryField's not standard behaviour (/django/
contrib/gis/forms/fields.py, line 31), it raises a ValidationError.
This is because GeometryField checks a "null" attribute in its clean()
method, instead of "required". "null" isn't defined in any of the
standard Django forms' fields. I suppose that it was consciously made
that way for some reason, but I don't think it's worth breaking the
standard behaviour for form fields.

I see two ways of fixing this:

- Introduce a backwards-incompatible change so that GeometryField
doesn't use "null", and use "required" instead. I don't know if this
has impact in other parts of Django.

- Allow for "null" OR "required". If null is True or required is False
then the field should be allowed to be blank. This should be backwards-
compatible AFAIK.

I hope I made myself clearer this time. Opinions please? If this
should be fixed, and I seriously think it must, I'll open a ticket and
attach a patch.

I see what you're talking about now, and I agree it's a bug.  Please
open up a ticket and we'll consider the design decisions there.
Thanks for the clarification.

-Justin

On 28 mar, 20:58, Justin Bronn <jbr...@gmail.com> wrote:

I did just that, it's ticket #10660: http://code.djangoproject.com/ticket/10660

I don't know if we could have this before 1.1, I'll make a patch as
soon as a design decision is reached.

Cheers.