form_for_instance generates empty choice for non-blank CharField with options

[Ilya Semenov <semenov@…>]

Given a model with a non-blank CharField with some options:

class Settings(models.Model):
    notification = models.CharField(maxlength=20, choices=(('foo','Foo'),('bar','Bar'))

create a form:

s = Settings.objects.get(...)
Form = forms.form_for_instance(s)
form = Form()

and display it in the template:

{{ form.notification }}

The select widget will have an empty choice "-----------", though the field doesn't allow blank values by its definition.

Another situation is when the field actually allows blanks, moreover, it provides its own blank value in the options:

class Settings(models.Model):
    notification = models.CharField(maxlength=20, blank=True, choices=(('foo','Foo'),('bar','Bar'),('', '- none of the above -'))

In this case, the select widget will also have an empty choice "-----------", and having two empty choices is quite confusing confusing.

The proposed patch resolves both issues.

[Brian Rosner <brosner@…>]

I am -1 on the first part. I think when a field w/ choices does not allow a blank value should display an empty value. This is true of a normal text input which has a default state of no value. This form validation would then correct the user by requesting them they fill out all required fields. I just think that selecting the first option in required drop-down is poor display behavior.

I am +1 on the second part. You should be allowed to override the default empty behavior of the select box.

[Ilya Semenov <semenov@…>]

This is true of a normal text input which has a default state of no value.

This is true for a normal text input (blank=False) only in a "new instance" form (returned by form_for_model), not when editing an existing instance (using form_for_instance). You'll never get an empty text input for a model instance with form_for_instance, since the instance has been fetched from the database and is supposedly valid.

I agree with the the argumentation for form_for_model, though, so the better patch is needed.

[Brian Rosner <brosner@…>]

Ah, yes, my bad. I didn't think my first one through. I see where you are coming from now. However I still feel like it doesn't matter that it is there. I think it can still be useful for providing a little more instructions about the select box. I think just removing the blank value for form_for_instance is too black magic for my liking.

[Ilya Semenov <semenov@…>]

The rationale behind my proposal is that according to the model contract, you can't change the value to be empty. By offering users the possibility to change the selection to "--------", we're misleading them. For example, see the "Component:" selector on Trac comments form below. There's no "empty" choice, since it wouldn't make sense.

If there was a good way to remove the blank values without changing the django core, I probably wouldn't post the ticket.
What I found is to directly access Form.base_fields[xxx].widget.options for each field, or write a custom formfield processor. Both ways feel a bit clumsy.

[Tai Lee]

I'm a strong -1 for this. Select fields should *always* have an empty option ('--------') unless the field is required *and* an explicit default value has been defined. The purpose of the empty option is to force users to make a selection by not providing an implicit default.

In the second case, oldforms used to show an empty option if the field was either not required, or was required but didn't have a default option. If newforms no longer works that way, we should fix that bug.

Assuming we reverted to the oldforms behaviour, a 'none of the above' option could be specified as ('none', '-- none of the above --') in your choices list. This wouldn't negate the need to force the user to make an actual selection, but if you want to remove the empty option and make 'none of the above' the default, just make your field required and set 'none' as the default.

Regarding the example of the 'Component:' selector in trac, this makes sense because an explicit default of 'Uncategorized' is defined.

[Ilya Semenov <semenov@…>]

The purpose of the empty option is to force users to make a selection by not providing an implicit default.

You're not reading carefully. That's not about forcing users to make a selection (as in case with form_for_model). That's about (visual) possibility to change the previously-made-selection to '--------'.

Consider a required field which three possible choices: A, B, C.
The form_for_model field (if there's no explicit default) should display '-----' as default and 3 choices below.
The form_for_instance field should display the previously-selected value as default and 2 other choices to change to.

Choosing a value and changing a value are different things.

[Simon Litchfield <simon@…>]

Either way, blank_choice (the label shown) needs to be an encouraged and easily settable option. Paying clients expect more than cryptic dashes for end users of real websites, eg 'select a car', or 'select your state'.

[mark.green@…]

I stumbled into the same issue (superfluous "-----" option) and here's
how I would like to see it fixed:

Add an option ("blank_choice"?) to form_for_model() and form_for_instance().

• If the user sets this option to an empty string then no dashes-option should be supplied.
• If the user sets this option to a non-empty string then that should be used instead of the dashes.
• If the user doesn't provide a value at all then I'd follow Ilya's advice; make form_for_model() include the dashes by default and form_for_instance() not.

I don't know how far the patch goes towards this goal but I agree with Simon that
this is a fairly important UI-issue, thus it'd be great if someone with sufficient
core-insight could beat the patch into shape and commit it soon.

[Brian Rosner]

First of all, adding options to form_for* helper functions that relate directly to UI of a widget should is not a good thing, let alone adding more options to them in general. If you want to change the display value for the empty string in a Select box then just use the formfield_callback kwarg to form_for_* helpers to override it. Here is an example that shows how to override the empty value display for both a foreign key and a CharField with choices.

models.py:

from django.db import models

class MyModel(models.Model):
    parent = models.ForeignKey(MyParentModel)
    car = models.CharField(max_length=20, choices=(('vw', 'VW'), ('toyota', 'Toyota')))

views.py:

from django import newforms as forms

def formfield_callback(f, **kwargs):
    if f.name == 'car':
        kwargs['widget'] = forms.Select(choices=f.get_choices(blank_choice=[(u'', u'Select a Car')]))
    if f.name == 'parent':
        kwargs['empty_label'] = u'Select a Parent'
    return f.formfield(**kwargs)

def some_view(request):
    obj = MyModel.objects.get(id=1)
    Form = form_for_instance(obj, formfield_callback=formfield_callback)

That is the general idea behind it. Make it work how you want.

[Ilya Semenov]

OK, just to make sure. Closing this ticket as "wontfix" states that the current django behaviour is considered the best within the suggested approaches.

That means, the following is explicitely and clearly meant:

1. If a field's choices= has an empty choice, like ("", "- No car -"), form_for_* displays two empty options.

2. If a field has blank=False, form_for_instance displays empty option anyway.

Do you completely realize the meaning of both statements, and still think that the described behaviour has well-thought logic behind it?

(To state my opinion, I think both of them make no sense.)

[Brian Rosner]

Yes, I do understand both items you discussed. I have tested my code and it does work.

1 . This is incorrect since using the blank_choice kwarg to the get_choices method override the default behavior. The reason why it displays two options is because you are defining the second one as you should not be.

2. If you need to utilize and enforce a blank=False on a field with choices then you should not be using a Select widget. You should rather be using a RadioSelect widget. Once again this falls back to utilizing formfield_callback. The fields in Django provide the common case and using the formfield_callback allows you to fix what is not working for you.

[Simon Litchfield <simon@…>]

I think form_for_ needs a whole new approach, a class-based one perhaps. One where we can provide all the extra functionality we need for DRY models <-> forms. Without adding a bajillion arguments to the outgrown form_for_ functions.

I've had clients specifically request those ugly dashes be removed, which isn't unreasonable. And I've had to resort to hackish solutions. In fact, most of my 'real world' usage of newforms with models (incl formsets) - for anything beyond a basic form - has ended up producing code that I think could be a lot simpler.

[Tai Lee <real.human@…>]

I agree, neither of those two options make sense. It is entirely reasonable for people to want their own "blank" option instead of the dashes, and to be able to position it at the end of the list rather than at the beginning. It is also entirely reasonable to use a Select list without an empty option. Where is it specified that only Radio buttons should be used where no blank option is allowed?

[anonymous]

I'm with semenov on this issue and don't agree with the "wontfix".
The solution for the "rename the dashes"-case seems feasible, it's a bit verbose but if it works - okay.
The proposed solution for the "suppress the dashes"-case is definately not acceptable.

You (brosner) argue, that RadioSelect should be used when no blank choice is needed.
Sorry, but this doesn't work out in the real world.

Imagine the standard case of a country-select field in a registration form. It is a mandatory field,
thus a blank option makes no sense. Also it must offer something in the ballpark of 240 choices.
You don't seriously propose to present 240 or more options as radio tick-boxes to the user?

Please provide a real fix or workaround to achieve the required functionality; a select-widget
that properly represents blank=False. We're not asking for eyecandy, this is a serious
usability issue.

[Brian Rosner]

Ok, I don't know if you guys understand how to read my example. I provided you with exactly the method of how to accomplish this. I am not saying my example is verbatim how to accomplish this. There are several areas that can be customized through the use of formfield_callback. Django does not provide every single possible method to fill the needs of a small majority of people that need a specific customization. I'll leave this as reopened and to be closed by a core developer who might see this my way.

Of course I don't see it reasonable to present 240 options as radio buttons. That was just a suggestion to accomplish it when you have a few options. The code I wrote in my comment above has all ways of changing the dang blank choice label however you please.

[Brian Rosner]

Ok, let me be even more verbose and also change my wording so you get my point. If you want to supress the empty choice, then look at the definition of get_choices in the Field class found in django.db.models.fields. It has an argument named include_blank by default it is True. Set this to False in the call to get_choices in the formfield_callback function to remove it entirely. The web browser will by default use the first item in the select box to be the default value.

[Tai Lee <real.human@…>]

brosner, What's the problem in having Django automatically suppress the empty choice, instead of requiring users to do it every time?

It's a simple useability issue. Select fields generated by form_for_instance which have blank=False should *never* contain a blank option. Just because there is a way to "work-around" a basic useability defect with callbacks doesn't mean that a fix should not be committed to Django so that no work-around is required.

This is not an edge case affecting a small minority of people needing a specific customisation. It's a basic mis-match between the Model and the default form generated by Django's helper functions.

I also think that it should be trivial for users to customise the value and position of the blank option when there is a blank option, but that seems to be a separate issue.

[mark.green@…]

I agree with Tai Lee.
No workarounds should be needed only to fix *obviously* wrong default behaviour.
Instead the default should be corrected and workarounds should be left to the people
who, for whatever strange reason, really want a blank choice on their blank=False fields.

I wonder why such an obvious bug spawns such a long discussion instead of just getting fixed...

[James Bennett]

Folks,

1. It's been pointed out that the default behavior -- display an empty choice -- has a rationale.
2. It's been pointed out that you can change the default behavior.
3. It's been pointed out how to change the default behavior.

At this point the only possible argument to be made is one which argues for flipping the default behavior in a backwards-incompatible way, which I would personally be against, and which really belongs on the mailing list, though it probably won't get anywhere; changing the default behavior to "never display an empty choice unless specifically told to", as you are loudly demanding, is flat-out dangerous (since it has the same effect as forcing every field with choices to have an arbitrary default based on the order of the choices list).

I'm closing wontfix again. DO NOT reopen this ticket unless changing the default behavior gains traction from core devs on the mailing list.

[Malcolm Tredinnick]

I'm reopening. There's some merit to the original request. However, since the patch includes neither tests nor documentation, it's not too surprising there was some confusion about it. Needs a much better patch, demonstrating behaviour for instances and models (non-instances) before it can go in.

Instances -- cases where a value has already been selected -- where blank=False can omit the blank value. There's actually another ticket open about this, too; discussing the same issue with US state fields. Possibly some of the discussion there is relevant, but I can't find the ticket now.

Finally, can people please think before they post. "I agree"/"I disagree" are not very useful comments, since the merit of a ticket isn't judged by popularity. If you have something extra to add to the discussion, add it. But if your point has already been made, you have nothing to add. So let's keep the noise down so that all the comments are actually relevant.

[Ilya Semenov]

If you need to utilize and enforce a blank=False on a field with choices then you should not be using a Select widget. You should rather be using a RadioSelect widget.

You must be kidding. See for example in Trac form below, Triage Stage selector. It's a select, not a radio-button select, and still it doesn't have "empty" choice, since it just wouldn't make sense.

I do realize that the programmer is able to override the default behaviour. The question is, if in the real world the "default" behaviour would be overriden 95% times, why not change the default? Why would anyone like to have an "empty" option for non-blank select in form_for_instance? Any reason, please.

[Ilya Semenov]

Attached the updated patch.

With this patch, blank option is not included if (and only if):

• a form_for_instance has a field with choices and blank=False
• a form_for_model has a field with choices, blank=False and explicit default=

The patch includes tests covering all the cases. The patch does not include the documentation, since the current form_for_* documentation doesn't mention the behaviour of any particular form fields.

[Ilya Semenov]

Moved new tests to separate section at the end of file

[Ilya Semenov]

(Sorry, formatting was lost in the previous post. Reposting the relevant part.)

With this patch, blank option is not included if (and only if):

• a form_for_instance has a field with choices and blank=False
  
• a form_for_model has a field with choices, blank=False and explicit default=

[Malcolm Tredinnick]

This isn't ready for checkin. It needs documentation to describe the behaviour. The fact that the current behaviour caused so many misunderstandings even in this ticket and the new behaviour is built on top of that, shows this need.

[Ilya Semenov]

Added docs describing the behaviour.

[Ilya Semenov]

See the revised patch with the documentation describing the behaviour.

(I also added myself to AUTHORS since of this ticket and #3955, #3032, #4086.)

[Ilya Semenov]

Reformulated the docs for form_for_instance()

[Malcolm Tredinnick]

(In [6733]) Fixed #4653 -- Improved the logic to decide when to include (and select as
initial value) the blank choice for a model field with choices. Thanks to
Ilya Semenov for persisting with this.

[jlpp]

In case anyone else has trouble with empty options, you might find this helpful:
​http://stackoverflow.com/questions/739260/customize-remove-django-select-box-blank-option