Allow 'True', 'False' and 'None' to resolve to corresponding Python objects

[anatoly techtonik <techtonik@…>]

[Original bug title: Invalid '==' expression silently ignored leading to invalid result]

Let me clarify the title - I am not sure the expression below is invalid. It is the expression I intuitively expect to work. But perhaps Django it is a specifics of template language. In any case it should either complain or produce expected output, which it doesn't.

_{{ closed }}_
{% if closed == True %} ***** {% endif %}

If closed variable is equal to None, the example above will output the string:

_None_ *****

Expected is to output nothing or to give a warning about invalid right-side argument.

If closed is boolean (not string) and is equal to True, it will output

_True_

Expected is to output

_True_ *****

or to give a warning about invalid right-side argument.

[Luke Plant]

Your example is invalid, because 'True' is not a literal that is understood by the template language. Rather, the string 'True' in your template is interpreted as a variable name. Since a variable called "True" does not exist in the context, the behaviour you see above follows (though the full sequence of steps is a little involved).

The expression you want is simply:

{% if closed %}

If you want the negation:

{% if not closed %}

(This, by the way, is normally considered good style in Python as well. Adding the no-op comparison == True is pointless - and so neither the majority of Python developers nor template writers would intuitively write this).

[Luke Plant]

By the way, the template language does not give warnings for missing variables. By design it succeeds silently, and on rare occasions throws exceptions.

[anatoly techtonik <techtonik@…>]

Replying to lukeplant:

Your example is invalid, because 'True' is not a literal that is understood by the template language.

This explains the behavior, but doesn't solve the issue.

Rather, the string 'True' in your template is interpreted as a variable name. Since a variable called "True" does not exist in the context, the behaviour you see above follows (though the full sequence of steps is a little involved).

Don't you think that Pythonic template engine should allow standard Python True/False keywords in comparison expressions? There is a clear need to use them for NullBooleanField comparisons, which is impossible otherwise (which is my case, btw, that your example below doesn't solve). The only reason not to teach Django understand True/False literals is to allow people set these variables in templates themselves, but I really don't think you'd like to encourage such practice.

The expression you want is simply:

{% if closed %}

If you want the negation:

{% if not closed %}

(This, by the way, is normally considered good style in Python as well. Adding the no-op comparison == True is pointless - and so neither the majority of Python developers nor template writers would intuitively write this).

That's not what I want. In my case closed is NullBooleanField used as a filter for issue list. None means that both open and closed issues should be returned. I need to distinguish between None and False.

[Simon Charette]

I know it might feel a bit hackish but I think you get work around this limitation by using the ​default_if_none filter.

{% if closed|default_if_none:'None' == 'None' %}
  None
{% else %}{% if closed %}
  True
{% else %}
  False
{% endif %}{% endif %}

[Luke Plant]

Your initial example only did a comparison to True, and didn't mention any comparison to None. I am not a mind reader!

The ticket has now changed in scope to the feature addition "Allow 'True', 'False' and 'None' to resolve to corresponding Python objects".

I can see why this would be useful, but I'm not entirely convinced, given that we've survived without these for a long time. Therefore marking DDN.

[Luke Plant]

The other thing to add is that comparison to 'None' does work (by accident, really). If you write:

{% if closed == None %}
  None
{% else %}{% if closed %}
  True
{% else %}
  False
{% endif %}{% endif %}

then you get the behaviour you want.

[anatoly techtonik <techtonik@…>]

Replying to lukeplant:

Your initial example only did a comparison to True, and didn't mention any comparison to None. I am not a mind reader!

I couldn't understand the magic behavior and where the bug is, because documentation doesn't warn that you can't use Python True/False literals in expressions that look like usual Python expressions (I also could not understand why would anyone limit people from using them). So, as a result of this report I expected the way to compare booleans to be ​at least documented if not fixed, because after spending several hours trying to debug this stuff, you start to understand people sending rays of hate to Django templating system. :)

My frustration with the fact that I don't understand how to compare booleans (when you can compare integers) is the reason I selected UI/UX checkbox several times. When I understood that it is not the boolean story alone, I split the user story into two. This one about True/False alone, and #17230 for None bug.

I can see why this would be useful, but I'm not entirely convinced, given that we've survived without these for a long time. Therefore marking DDN.

I anticipated such reaction when it became clear in #17230 that the bug is actually there and was there for a long time, so thanks for not closing it immediately as 'works for me'.

​My workaround (which I feel is still hackish and not really intuitive) is:

  {%if closed|lower == 'false' %} (opened only) {%endif%}
  {%if closed|lower == 'true' %} (closed only) {%endif%}

[Aymeric Augustin]

I classified Python builtins with the script below and I can confirm that True, False and None are the only ones that would make sense in templates. There's no risk of feature creep here.

The Django template language is primarily designed for designers, but it's also widely used by Python programmers. The principle of least surprise tells that we should make these three values builtins of the template language too. It just feels wrong that True evaluates silently to None.

---

def qualify(thing):
    if isinstance(thing, BaseException):
        return 'exc'
    elif type(thing) == type:
        return 'cls'
    elif type(thing) == type(map):
        return 'fun'
    else:
        return 'other: ' + type(thing).__name__

for b in dir(__builtins__):
    print '%30s  %s' % (b, qualify(getattr(__builtins__, b)))

[Aymeric Augustin]

I'm attaching a patch showing one way to implement this.

[Aymeric Augustin]

Don't forget to kill the note here: ​https://docs.djangoproject.com/en/dev/topics/i18n/timezones/#timezone

[Aymeric Augustin]

carljm> you have my agreement on #17229, I think it has no downsides and fixes something that is likely to surprise (from #django-dev)

[Aymeric Augustin]

This is arguably a new feature of the template engine, so it'll have to wait until 1.4 is out.

[anatoly techtonik <techtonik@…>]

Does that mean it won't be included in 1.4? Oh my.

[Aymeric Augustin]

Well -- it could have been if you had provided a suitable patch (with tests and docs).

[anatoly techtonik <techtonik@…>]

Well, if it wasn't a bug in the first place - I probably felt too lazy even to submit it. =)

[Aymeric Augustin]

In [17894]:

Fixed #17229 -- Allow 'True', 'False' and 'None' to resolve to the corresponding Python objects in templates.

[anatoly techtonik <techtonik@…>]

Awesome. Nice to see it is going to be in Django 1.5

[Claude Paroz]

Changeset r17894 produced several errors in the Django test suite. In fact, it reveals issues where Context instances are initialized with a Context argument instead of a simple dict.
Examples:

• In browser:django/trunk/django/contrib/auth/tests/urls.py, several render_to_response calls have the Context instance as second argument, which seems to be plain bugs (uncaught until now)
• In browser:django/trunk/django/contrib/admin/templatetags/admin_modify.py, the prepopulated_fields_js inclusion tag returns a Context instance instead of a dict.

The question is now whether Context initialization should accept other Context instance as the __init__ dict_ parameter (and then this changeset should be fixed) or if we should enforce dict_ as a dict and fix places in Django code when it is not the case. I'd be in favour of the latter, but this might need some more investigation.

[Anssi Kääriäinen]

It could make sense to just revert r17894 to make trunk work again.

Otherwise, the question is how likely user code is to pass in Context instances. I guess it happens, as it wasn't exactly uncommon in our code either. So, if Context is going to be disallowed, then at least provide a clear error message of what happened.

[Aymeric Augustin]

The regression is tracked in #18103.

[Aymeric Augustin]

Replying to claudep:

Changeset r17894 produced several errors in the Django test suite. In fact, it reveals issues where Context instances are initialized with a Context argument instead of a simple dict.

Claude, I've moved this to its own ticket (#18105) because I'd like to fix the regression ASAP, but keep track of your suggestion.