[Django] #19210: django.utils.timesince() does not account for leap years
Django
-------------------------------+-----------------------
Reporter: jnns | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Severity: Normal | Keywords: date time
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------
The output of `django.utils.timesince()` will become increasingly
inaccurate as the number of leap years in the supplied time period grows.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jnns | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* component: Uncategorized => Core (Other)
* needs_tests: => 0
* needs_docs: => 0
Comment:
Using 365.2425 instead of 365 in `chunks` might give better results for
long periods, without much distortion for shorter periods.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:1>
Django
Reporter: jnns | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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
----------------------------------------------------------------------
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:2>
Django
Reporter: jnns | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hirokiky):
Fixed test not to use dateutil
https://github.com/hirokiky/django/commit/b3b81594fc6c0a424c414ffdc751e01c55e11cb1
Because, other sources doesn't use it.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:3>
Django
Reporter: jnns | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hirokiky):
Solved with dirty implemantion
https://github.com/hirokiky/django/commit/f581659080e78891d043ecbd63e2fc349c786c44
It seems good to separate the resolusion depending on whether the unit is
year or not.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:4>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hirokiky):
* status: new => assigned
* cc: hirokiky@… (added)
* owner: nobody => hirokiky
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:5>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hirokiky):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:6>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hirokiky):
I send pull-request as https://github.com/django/django/pull/622 .
Please check it, if you do not mind.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:7>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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?
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:9>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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).
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:10>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:11>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:12>
Django
Reporter: jnns | Owner: hirokiky
Type: Bug | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:13>
Django
Reporter: jnns | Owner:
Type: Bug | Status: new
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by hirokiky):
* status: assigned => new
* owner: hirokiky =>
Comment:
Thanks @susan @timo. I can't take time for this problem, so I'm leaving.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:14>
Django
Reporter: jnns | Owner:
Type: Bug | Status: new
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by aaugustin):
For what it's worth, I consider that timesince returns an order of
magnitude for human consumption and a small inaccuracy isn't critical. I
wouldn't mind if this ticket was closed as wontfix.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:15>
Django
Reporter: jnns | Owner:
Type: Bug | Status: new
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by jnns):
What about the use case that I want to display the age of a person given
his or her birthdate? I consider this an order of magnitude for human
consumption but `timesince` will be displaying an incorrect number of
years.
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:16>
Django
Reporter: jnns | Owner: raphaelm
Type: Bug | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by raphaelm):
* owner: => raphaelm
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:17>
Django
Reporter: jnns | Owner: raphaelm
Type: Bug | Status: assigned
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by raphaelm):
* needs_better_patch: 1 => 0
Comment:
After looking into a lot of algorithms for date diffing, including the
ones suggested above and the ones from `python-dateutil` (which get pretty
complex) I decided to go for the most simple solution I could find. I
assume `timesince` wasn't ever meant to be an calculation. If we would
want an exact calculation, we should completely re-implement it to
correctly work with the different number of days in the months and so on.
However, there is a very simple fix to the accumulated error that grows
with the given time period: just use `calendar.leapdays` from the standard
library and substract it from the number of days.
I created a pull request including a regression test:
https://github.com/django/django/pull/4785
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:18>
Django
Reporter: jnns | Owner: raphaelm
Component: Utilities | Version: master
Severity: Normal | Resolution: fixed
Keywords: date time | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6700c909358d2ff9ff96f28dbc549906d8d32206" 6700c90]:
{{{
#!CommitTicketReference repository=""
revision="6700c909358d2ff9ff96f28dbc549906d8d32206"
Fixed #19210 -- Added leap year support to django.utils.timesince()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19210#comment:19>