Code

Opened 21 months ago

Last modified 3 months ago

#19210 new Bug

django.utils.timesince() does not account for leap years

Reported by: jnns Owned by:
Component: Utilities Version: master
Severity: Normal Keywords: date time
Cc: hirokiky@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The output of django.utils.timesince() will become increasingly inaccurate as the number of leap years in the supplied time period grows.

Attachments (1)

19210_test.patch (734 bytes) - added by hirokiky 19 months ago.
Added test to verifiy the behavior while long time.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 21 months ago by claudep

  • Component changed from Uncategorized to Core (Other)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Using 365.2425 instead of 365 in chunks might give better results for long periods, without much distortion for shorter periods.

Changed 19 months ago by hirokiky

Added test to verifiy the behavior while long time.

comment:2 Changed 19 months ago by hirokiky

I applied my patch and tried test, using 365.2425 instead of 365 in chunks.
test_thousand_years_ago passed, but a test failed, like this:

======================================================================
FAIL: test_other_units (regressiontests.utils.timesince.TimesinceTests)
Test other units.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/hirokiky/django/django/tests/regressiontests/utils/timesince.py", line 42, in test_other_units
    self.assertEqual(timesince(self.t, self.t+self.oneyear), '1 year')
AssertionError: u'12 months' != u'1 year'
- 12 months
+ 1 year


----------------------------------------------------------------------

comment:3 Changed 19 months ago by hirokiky

Fixed test not to use dateutil https://github.com/hirokiky/django/commit/b3b81594fc6c0a424c414ffdc751e01c55e11cb1
Because, other sources doesn't use it.

comment:4 Changed 19 months ago by hirokiky

Solved with dirty implemantion https://github.com/hirokiky/django/commit/f581659080e78891d043ecbd63e2fc349c786c44
And commited to fix this commit https://github.com/hirokiky/django/commit/68d64fe91a73dbf4bebda98ebee0bb7b8b084ab6

It seems good to separate the resolusion depending on whether the unit is year or not.

Last edited 19 months ago by hirokiky (previous) (diff)

comment:5 Changed 19 months ago by hirokiky

  • Cc hirokiky@… added
  • Owner changed from nobody to hirokiky
  • Status changed from new to assigned

comment:6 Changed 18 months ago by hirokiky

  • Has patch set

comment:7 Changed 18 months ago by hirokiky

I send pull-request as https://github.com/django/django/pull/622 .

Please check it, if you do not mind.

comment:8 Changed 16 months ago by aaugustin

  • Component changed from Core (Other) to Utilities

comment:9 Changed 12 months ago by susan

@hirokiky, I've applied your PR branch and there are errors in the utils_tests test suite. See stack trace below:

======================================================================
FAIL: test_date_objects (utils_tests.test_timesince.TimesinceTests)
Both timesince and timeuntil should work on date objects (#17937).


Traceback (most recent call last):
../Projects/django/tests/utils_tests/test_timesince.py", line 112, in test_date_objects

self.assertEqual(timesince(today + self.oneday), '0\xa0minutes')

AssertionError: u'0 minutes' != u'0\xa0minutes'

  • 0 minutes

?
+ 0\xa0minutes
?

======================================================================
FAIL: test_different_timezones (utils_tests.test_timesince.TimesinceTests)
When using two different timezones.


Traceback (most recent call last):
../Projects/django/tests/utils_tests/test_timesince.py", line 105, in test_different_timezones

self.assertEqual(timesince(now), '0\xa0minutes')

AssertionError: u'0 minutes' != u'0\xa0minutes'

  • 0 minutes

?
+ 0\xa0minutes
?

======================================================================
FAIL: test_display_second_before_first (utils_tests.test_timesince.TimesinceTests)


Traceback (most recent call last):
../Projects/django/tests/utils_tests/test_timesince.py", line 74, in test_display_second_before_first

'0\xa0minutes')

AssertionError: u'0 minutes' != u'0\xa0minutes'

  • 0 minutes

?
+ 0\xa0minutes
?

======================================================================
FAIL: test_equal_datetimes (utils_tests.test_timesince.TimesinceTests)
equal datetimes.


Traceback (most recent call last):
../Projects/django/tests/utils_tests/test_timesince.py", line 25, in test_equal_datetimes

self.assertEqual(timesince(self.t, self.t), '0\xa0minutes')

AssertionError: u'0 minutes' != u'0\xa0minutes'

  • 0 minutes

?
+ 0\xa0minutes
?

======================================================================
FAIL: test_ignore_microseconds_and_seconds (utils_tests.test_timesince.TimesinceTests)
Microseconds and seconds are ignored.


Traceback (most recent call last):
../Projects/django/tests/utils_tests/test_timesince.py", line 30, in test_ignore_microseconds_and_seconds

'0\xa0minutes')

AssertionError: u'0 minutes' != u'0\xa0minutes'

  • 0 minutes

?
+ 0\xa0minutes
?

======================================================================
FAIL: test_naive_datetime_with_tzinfo_attribute (utils_tests.test_timesince.TimesinceTests)


Traceback (most recent call last):
.../django/tests/utils_tests/test_timesince.py", line 127, in test_naive_datetime_with_tzinfo_attribute

self.assertEqual(timesince(future), '0\xa0minutes')

AssertionError: u'0 minutes' != u'0\xa0minutes'

  • 0 minutes

?
+ 0\xa0minutes
?


Ran 248 tests in 0.473s

FAILED (failures=6, errors=6)

Did anyone else also get errors? Or did all the tests pass?

comment:10 Changed 12 months ago by timo

Susan, it sounds like you may be running the latest version of the tests with a version of Django before 7d77e9786a118dd95a268872dd9d36664066b96a (or perhaps you didn't apply the pull request quite right).

comment:11 Changed 12 months ago by susan

The 6-mo old patch was created before that new commit you're referring to, timo. That means the patch needs to be updated.

comment:12 Changed 12 months ago by susan

I partially updated the patch on my origin branch here: https://github.com/onceuponatimeforever/django/compare/pull_622. It's not a PR, because there are still errors. I say "partially updated", because when I run the utils_tests, I get a dozen errors related to this:

 File "../django/django/utils/timesince.py", line 65, in timesince
    s = ugettext('%(number)d %(type)s') % {'number': count, 'type': name(count)}
TypeError: '__proxy__' object is not callable

I'm not sure how to fix this error. But I'll leave it up to the next contributor to take a look at this.

comment:13 Changed 10 months ago by timo

  • Patch needs improvement set

comment:14 Changed 3 months ago by hirokiky

  • Owner hirokiky deleted
  • Status changed from assigned to new

Thanks @susan @timo. I can't take time for this problem, so I'm leaving.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from (none) to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


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

 
Note: See TracTickets for help on using tickets.