GeometryField doesn't honor the required attribute

[Fidel Ramos]

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.

[Justin Bronn]

On Mar 25, 12:20 pm, Fidel Ramos <fidelra...@gmail.com> wrote:
> My proposed solution would be to call Field's clean() instead of
> checking self.null.

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

[Fidel Ramos]

On 27 mar, 19:49, Justin Bronn <jbr...@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.

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.

[Justin Bronn]

> 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

[Fidel Ramos]

On 28 mar, 20:58, Justin Bronn <jbr...@gmail.com> wrote:
> 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.

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.