timesince template filter reverses its parameters when given a base date

[Andrew Shearer <ashearerw@…>]

The timesince template filter appears to swap its parameters when given a base date argument, leading to a typical result of "0 minutes" instead of the expected time difference. This makes it a synonym of the timeuntil filter. Omitting the optional argument makes both filters work as expected.

The django.utils.timesince function itself works correctly. The timesince and timeuntil template filters both call it with an identical line of code, explaining their matching results.

A patch is attached that un-flips the parameters and adds test coverage of the base date argument to the filters.

[Andrew Shearer <andrew@…>]

Patch

[Andrew Shearer <andrew@…>]

Failing test result before applying the patch:

File "tests.py", line 354, in __main__
Failed example:
    timesince(datetime.datetime(2005, 12, 29), datetime.datetime(2005, 12, 30))
Expected:
    u'1 day'
Got:
    u'0 minutes'

[Simon Greenhill]

(that was me)

[Russell Keith-Magee]

Replying to Andrew Shearer <andrew@ashearer.com>:

Patch breaks the template filter test for timesince. The break occurs because the provided example of bad behaviour is incorrect:

Failed example:

timesince(datetime.datetime(2005, 12, 29), datetime.datetime(2005, 12, 30))

Expected:

u'1 day'

Got:

u'0 minutes'

29 Dec is before 30 december (the now argument on the timesince filter) - therefore 0 minutes is returned. The parameter ordering is slightly different to the timesince utility method.

[ashearer]

I've re-read the documentation, and it seems to clearly contradict the current behavior. At least two of the three relevant sentences contradict it, while the remaining sentence supports it. If nothing else, the documentation should be changed.

But the current behavior still doesn't make much sense. Here's the output of a test to illustrate that.

!python
context = {
        'now': datetime.now(),
        'earlier': datetime.now() - timedelta(days=7),
        'later': datetime.now() + timedelta(days=7),
}

Template output:

• earlier|timesince: 1 week
• earlier|timesince:now: 0 minutes
• earlier|timeuntil: 0 minutes
• earlier|timeuntil:now: 0 minutes
• later|timesince: 0 minutes
• later|timesince:now: 1 week
• later|timeuntil: 1 week
• later|timeuntil:now: 1 week

Problem 1: mydate|timesince:now == mydate|timeuntil:now. The two-argument forms of timesince and timeuntil are synonyms, and therefore redundant. But their names, documentation, and single-argument forms suggest that they should give opposite results.

Problem 2: mydate|timesince != mydate|timesince:now. According to its documentation, timesince "[t]akes an optional argument that is a variable containing the date to use as the comparison point (without the argument, the comparison point is now)." But passing now explicitly as the comparison point gives opposite results from letting the comparison point default to now. (The timeuntil documentation contains almost exactly the same language, but passing now explicitly works the same as letting it default implicitly.)

Problem 3: The timesince documentation also says that '“0 minutes” will be returned for any date that is in the future relative to the comparison point.' The previous paragraph says that the optional argument is the comparison point. This corresponds to the 'later|timesince:now' line above, which returns '1 week' instead of the '0 minutes' documented.

In summary, the timesince filter is at best confusing (in which I have company at ticket #8453) and at worst flies in the face of analogies to timeuntil, its own single argument form, and two out of the three sentences of documentation that mention its two-argument form.

(By the way, russelm: the parameter ordering is "slightly" different? There are only two parameters. Which ordering would rise to the level of "extremely" different?)

[Russell Keith-Magee]

Replying to ashearer:

In summary, the timesince filter is at best confusing (in which I have company at ticket #8453) and at worst flies in the face of analogies to timeuntil, its own single argument form, and two out of the three sentences of documentation that mention its two-argument form.

Now that you've provided a set of test cases and highlighted the exact problem, I can see what you're driving at. If your original patch had these 8 examples (and then explained why the patch broke the three existing test cases), it would have been accepted much faster.

I'll update the patch and tests and commit this in the near future.

(By the way, russelm: the parameter ordering is "slightly" different? There are only two parameters. Which ordering would rise to the level of "extremely" different?)

You know - pedantry has a lot more impact when you take the care to correctly spell the name of the person you are criticising :-)

[Russell Keith-Magee]

(In [8481]) Fixed #7443: Corrected a long standing mistake in the timesince/timeuntil filters when using a parameter for 'now'. Thanks to Andrew Shearer <ashearerw@…> for the report.

[Russell Keith-Magee]

I wasn't aware of #8453, which makes it seem like there has been some other discussions around this topic. I've reverted the change until I work out what is going on here. I'll attach the patch that I reverted in case we need to apply it again.

[Russell Keith-Magee]

Patch applied as [8481] but then reverted.

[Jeremy Carbaugh]

Just wanted to point out that this issue was also fixed in #7201 which fixes a timezone issue with timesince and timeuntil. I wasn't aware of this patch or #8453 when I fixed #7201 at the DC sprint. If fixing timesince in this patch causes backwards-incompatible behavior, it would make sense to commit it with #7201 as that also has backwards-incompatible changes.

[Russell Keith-Magee]

(In [8535]) Fixed #7743: Reverted [8483], which was itself a reversion of [8481], after confirmation from Malcolm. Corrected a long standing mistake in the timesince/timeuntil filters when using a parameter for 'now'. Thanks to Andrew Shearer <ashearerw@…> for the report.

Fat fingers stuffed up the commit message in SVN.