The 'add' template filter only works for integers, and can fail noisily

[Bradley Ayers]

The 'add' template tag attempts to convert both arguments to ints using int() and there's no try/except around it (thus making it noisy if you try and add two objects which are not ints). In addition, there's no reason why it should be limited to only adding together integers, it should be able to support any two objects that can be added together.

[Filip Gruszczyński]

You mean 'add' filter, not the tag right?

Allowing any two objects to be added together isn't that easy. If you use this filter on an object that can be sumed with an int, the filter must somehow know to coerce the argument to int. And what if you want to add a string to a string? Then no coercion should be be applied. And if there was a list or tuple it would have to be turned into one.

Maybe some simpler filter, that would accept some several base types (int, string, list, tuple, set, dict) and allowing adding one to another. If it couldn't turn the argument into something acceptable, it would fail silently and return object on which filter was applied.

[Filip Gruszczyński]

Maybe something like this:

def add(value, arg):
    """Adds the arg to the value."""
    if isintnace(value, string):
		return value + arg
	elif isinstance(value, int):
		try:
			return value + int(arg)
		except TypeError:
			return value
	elif isinstance(value, list):
		try:
			return value + list(arg.split(','))
		except AttributeError:
			return value
	elif isinstance(value, tuple):
		try:
			return value + tuple(arg.split(','))
		except AttributeError:
			return value
	return value

Of course it can be extended a little more.

[dc]

I think better not convert types at all (explicit is better than implicit) and use duck typing.

def add(value, arg):
    try:
        return value + arg
    except TypeError:
        return value

This will work for any object with __add__ or __radd__ method.

[Filip Gruszczyński]

This won't work with ints, because value is either an int (and will raise an exception) or a string (and it will concatenate those string).

Furthermore it couldn't be used in following situtation:

x = []

...

x|add:'5,6,7'

which for some could be useful.

[Filip Gruszczyński]

OK, I don't know, whether add should have any more functionality, but it certainly needs to be silenced and proper tests should be written. I am posting a patch with just that.

[Chris Beaven]

I agree with dc; don't worry with a load of extra functionality (but IMO it should handle non-ints).

The try/except may as well be a catch-all, too, rather than TypeError or ValueError specific.

Hrm... to be totally backwards compatible, I guess that we'd need to continue with the current behaviour of trying to cast to int first (in case people are relying on adding string-integers), then fall back to a ducktype add.
That's pretty annoying if you wanted to add floats/decimals though.

I'm going to push to a design decision regarding which direction to go. Bring it up on the google dev group if you want some more discussion.

[Chris Beaven]

Non-int adding would be useful for #11689 - if the non-int part of this ticket is accepted, that can be closed as a duplicate.

[Filip Gruszczyński]

I don't like sucking all exceptions and this shouldn't be done. Imagine there is a custom object, that can be cast to an int and during coercing it will raise a different exception. Why should user lose this information? This is completely contrary to python zen.

When it comes to ducktyping, you have to remember, that the object on which filter is used, will have to able to accept a string as an argument of add. Most basic types:

In [1]: [] + ''
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/gruszczy/<ipython console> in <module>()

TypeError: can only concatenate list (not "str") to list

In [2]: () + ''
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/gruszczy/<ipython console> in <module>()

TypeError: can only concatenate tuple (not "str") to tuple

just can't do this.

[Chris Beaven]

The user should lose this information because a filter should never raise an exception.
Read the first paragraphs here: ​http://docs.djangoproject.com/en/dev/howto/custom-template-tags/#writing-custom-template-filters

I understand ducktyping.
A filter doesn't only accept strings as an argument, it could be any object type. If they can't be added together, then it should just fail silently - that's the template author's problem. This is not javascript, so no automatic casting.

[Filip Gruszczyński]

If they should never raise an exception, then indeed it should be sucking all exceptions.

The filter obviously accepts all objects, because that's just how Python works. However, if used in template only strings will be passed into such filter. Unless object on which filter is run can somehow use string in add, you won't get anything useful. Neither integers, nor lists/tuples can do that. Could you give some example of objects from stdlib/django that could benefit from using add with a string, that aren't strings?

[Chris Beaven]

I'm not sure where your delusion comes from that a template only deals with strings. I already gave a perfect example of a real life useful non-string addition in a template: see ticket #11689.

Here's another arbitrary example:

{% for obj in obj.list_a|add:obj2.list_b %}...{% endfor %}

[Filip Gruszczyński]

It was truly a delusion, because I had no idea that filter could work like this. If so, I'll provide proper patch and tests for a few different cases.

[Chris Beaven]

You may have had that view because Django templates always applie unicode() to the final output of variable tags. Glad we got to the conclusion in the end :)

I'll review your patch when it comes in and push forwards if it's all good. Remember, we need to keep the current behaviour so it should look something like this:

try:
    value, arg = int(value), int(arg)
except (TypeError, ValueError):
    pass
try:
    return value + arg
except:
    return value

[Filip Gruszczyński]

Yup, I know. The code itself is rather short, I just need to write some more tests to keep it safe. There is only one thing that isn't very nice - if someone wants to concatenate '123' and '456', he will get 579, which might not be exactly the thing he wants. I'll write the docs too.

[Filip Gruszczyński]

coercing to int, then ducktyping + tests + docs

[Filip Gruszczyński]

OK, done.

[Chris Beaven]

Looking good.

[Johannes Dollinger]

See #10143.

[Jacob]

(In [12497]) Fixed #11687: the add filter is now less failsome when faced with things that can't be coerced to integers.

[Jacob]

(In [12499]) [1.1.X] Fixed #11687: the add filter is now less failsome when faced with things that can't be coerced to integers.

Backport of [12497] from trunk, although the fix here is slightly different to
avoid adding new behavior to a bugfix branch.

[Jacob]

Milestone 1.2 deleted