Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15791 closed New feature (fixed)

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

Reported by: ejucovy Owned by: nobody
Component: Template system Version: master
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 ejucovy 3 years ago.
do_not_call_with_tests.diff (7.7 KB) - added by ejucovy 3 years ago.
same patch, with tests

Download all attachments as: .zip

Change History (12)

Changed 3 years ago by ejucovy

Changed 3 years ago by ejucovy

same patch, with tests

comment:1 Changed 3 years ago by ejucovy

  • Cc ethan.jucovy@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 3 years ago by ejucovy

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 Changed 3 years ago by lukeplant

  • Triage Stage changed from Unreviewed to Ready 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 Changed 3 years ago by ejucovy

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 Changed 3 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

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 Changed 3 years ago by lukeplant

@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 follow-ups: Changed 3 years ago by akonsu@…

  • Easy pickings unset
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • 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

comment:8 in reply to: ↑ 7 Changed 3 years ago by ejucovy

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.

comment:9 in reply to: ↑ 7 Changed 3 years ago by anonymous

  • Resolution set to fixed
  • Status changed from reopened to closed

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 Changed 3 years ago by rosarior

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.