timesince template filter reverses its parameters when given a base date

Patch

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'

(that was me)

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.

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?)

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 :-)

(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@shearersoftware.com> for the report.

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.

Patch applied as [8481] but then reverted.

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.

(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@shearersoftware.com> for the report.

Fat fingers stuffed up the commit message in SVN.