Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#15791 closed New feature (fixed)

New feature: callable objects can signal that templates should never call them

Reported by: Ethan Jucovy Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: ethan.jucovy@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a Python class that contains some data, and also has a __call__ method, e.g.:

class Doodad(object):
    def __init__(self, x, y):
        self.x = x
        self.y = y

    def __call__(self, a, b):
        return (self.x + a, self.y + b)

I'd like to be able to use instances of this class in my templates, to reference their attributes:

from django.template import Template, Context

ctx = Context(dict(my_doodad=Doodad(5, 10)))
Template("My doodad's x value is {{my_doodad.x}}").render(ctx)

But because the object has a __call__ method which takes more than zero arguments, Django's template system will try to call the method, catch the resulting TypeError, and replace the object with settings.TEMPLATE_STRING_IF_INVALID.

With the attached patch, callable objects can set an attribute to signal that Django's template system should leave them alone altogether.

This is analogous to the alters_data attribute, which signals that Django's template system should replace the object with settings.TEMPLATE_STRING_IF_INVALID without trying to call it.

Attachments (2)

do_not_call_in_templates.diff (2.2 KB ) - added by Ethan Jucovy 13 years ago.
do_not_call_with_tests.diff (7.7 KB ) - added by Ethan Jucovy 13 years ago.
same patch, with tests

Download all attachments as: .zip

Change History (12)

by Ethan Jucovy, 13 years ago

by Ethan Jucovy, 13 years ago

Attachment: do_not_call_with_tests.diff added

same patch, with tests

comment:1 by Ethan Jucovy, 13 years ago

Cc: ethan.jucovy@… added

I attached a new version of the patch that includes tests for this behavior, as well as the implementation and docs. I added a new module + test suite in regressiontests/templates since there didn't seem to be any existing place to put it.

This patch also includes tests for the existing behavior (implicitly calling variables by default, and discarding variables with alters_data=True) -- there don't appear to be any existing tests for these behaviors in Django's test suite, as far as I can tell.

comment:2 by Ethan Jucovy, 13 years ago

It would be nice if there were also a way to signal this from the templates themselves, perhaps with a {{variable|nocall}} filter or a {% nocall %} .. {% endnocall %} block tag. I'm not sure if the former is even possible (since the variable would usually be called before the filter is applied) but the latter might be. A template-driven approach would have the advantage of not requiring direct access to the object's attributes, which can be inconvenient in some circumstances (e.g. fetching large batches of callable objects whose interface is defined in a third-party library)

comment:3 by Luke Plant, 13 years ago

Triage Stage: UnreviewedReady for checkin

It's unfortunate that we need this, as it seems a bit hacky.

However, it is a genuine problem. An alternative solution is to convert callable objects to non-callable by some magic, so that the template system does not need to be altered. But that seems just as bad, and will probably have more gotchas.

Your solution follows the precedent of alters_data, while being orthogonal to it (which I didn't realise at first), in that it allows attributes of the callable to be accessed, while alters_data does not.

The 'nocall' filter or template tag would be much more work, and harder to implement, and I don't think are practical. For large batches of callable objects retrieved by some third-party library, I think a good enough workaround is to wrap the data in a simple generator that adds the 'do_not_call_in_templates' attribute to instances where needed:

def mark_not_callable(objects):
    for obj in objects:
        obj.do_not_call_in_templates = True
        # or possibly:
        obj.some_sub_object.do_not_call_in_templates = True
        yield obj

def view(request):
    # ...
    context['objects'] = mark_not_callable(some_objects)

'do_not_call_in_templates' seems a bit verbose, but it's explicit and it's your bike-shed, so we'll leave that.

Thank you for the tests for alters_data. There are tests for other callables in class 'Templates' in regressiontests/templates/tests.py, but not for alters_data that I can find, and the nature of the tests means it is good to explicitly test that the methods are not called, as you have done. The simple test_callable test you have added can be left, because it uses a class with a __call__ method, while the existing test uses a lambda, and its conceivable that having both types of callable tested could be useful.

For future reference, I found one tiny nit, which I'll fix when I commit - our coding style for templates is to do {{ variable }} and not {{variable}}.

So, for all these reasons, marking accepted, and in fact, ready for checkin. Thank you very much!

comment:4 by Ethan Jucovy, 13 years ago

Excellent, thanks!

I'm definitely not wedded to do_not_call_in_templates for the name -- I just wasn't able to think of anything shorter that was both coherent & self-documenting. If you can think of an alternative name, by all means go ahead. :)

comment:5 by Luke Plant, 13 years ago

Resolution: fixed
Status: newclosed

In [16045]:

Fixed #15791 - method to signal that callable objects should not be called in templates

Thanks to ejucovy for the suggestion and patch!

comment:6 by Luke Plant, 13 years ago

@ejucovy:

For your information, I made some other tweaks to the patch, including:

  1. Fixed in an error in the docs, which talked about the empty string rather than TEMPLATE_STRING_IF_INVALID
  2. Clarified that the do_no_call_in_templates attribute should be set to True
  3. Added a 'versionadded' directive
  4. Removed/trimmed some of the comments in the tests - they were a little bit verbose for my taste. Comments should not duplicate information that is self-evident from the code.

Thanks again.

comment:7 by akonsu@…, 13 years ago

Easy pickings: unset
Resolution: fixed
Status: closedreopened
UI/UX: unset

this attribute is an ugly hack. the original code can easily be fixed without using this attribute:

from django.template import Template, Context

ctx = Context(dict(my_doodad=lambda : Doodad(5, 10)))
Template("My doodad's x value is {{my_doodad.x}}").render(ctx)

please consider reverting

in reply to:  7 comment:8 by Ethan Jucovy, 13 years ago

Replying to akonsu@…:

this attribute is an ugly hack. the original code can easily be fixed without using this attribute:

from django.template import Template, Context

ctx = Context(dict(my_doodad=lambda : Doodad(5, 10)))
Template("My doodad's x value is {{my_doodad.x}}").render(ctx)

please consider reverting

Ah, I hadn't thought of that -- clever, thanks! But surely this is an even more ugly hack?

The benefit of an attribute is that it can be both findably documented (I searched for existing workarounds and Google didn't turn anything up) and self-documenting. If I see ctx = Context(dict(my_doodad=lambda : my_doodad)) in code I will have no idea why that's being done (even with the context it took me a minute to understand why that's a successful workaround) -- but if I see my_doodad.do_not_call_in_templates = True; ctx = Context(dict(my_doodad=my_doodad)) it's perfectly clear.

It's also reusable lower down -- if I have control over the Doodad class definition, want to retain a __call__ API, and know it's likely to be used in Django templates, I can set the attribute on the class, and view authors need not worry about it at all. I can think of no corresponding way to do that with your solution (short of removing or rewriting __call__), instead the view author has to be mindful of this particular edge case in the template system.

in reply to:  7 comment:9 by anonymous, 13 years ago

Resolution: fixed
Status: reopenedclosed

Replying to akonsu@…:

this attribute is an ugly hack. the original code can easily be fixed without using this attribute [...] please consider reverting

Please open another ticket with your proposal, or even better start a thread in the django-dev mailing list.

Restoring the ticket status.

comment:10 by Roberto Rosario, 13 years ago

How to sidestep the issue until Django 1.4 is released:

    def encapsulate(func):
        return lambda: func

http://stackoverflow.com/questions/6861601/cannot-resolve-callable-context-variable

Note: See TracTickets for help on using tickets.
Back to Top