[Django] #30141: dateparse parse_duration to accept negative durations
Django
-----------------------------------------+------------------------
Reporter: heikkimattila | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-----------------------------------------+------------------------
The https://docs.djangoproject.com/en/2.1/_modules/django/utils/dateparse/
defines:
{{{
standard_duration_re = re.compile(
r'^'
r'(?:(?P<days>-?\d+) (days?, )?)?'
r'((?:(?P<hours>-?\d+):)(?=\d+:\d+))?'
r'(?:(?P<minutes>-?\d+):)?'
r'(?P<seconds>-?\d+)'
r'(?:\.(?P<microseconds>\d{1,6})\d{0,6})?'
r'$'
)
}}}
that doesn't match to negative durations, because of the <hours>
definition final (lookahead) part does not have '-?' in it. The following
will work:
{{{
r'((?:(?P<hours>-?\d+):)(?=-?\d+:-?\d+))?'
}}}
(Thanks to Konstantin Senichev for finding the fix.)
--
Ticket URL: <https://code.djangoproject.com/ticket/30141>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: heikkimattila | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+--------------------------------------
Changes (by Tim Graham):
* type: Uncategorized => Bug
* component: Uncategorized => Utilities
* easy: 1 => 0
Comment:
Please give an example valid that's not working. There are
[https://github.com/django/django/blob/14581553e4ce9918aeb427fee7e5d7b6681e4e85/tests/utils_tests/test_dateparse.py#L112-L122
some tests for negative values].
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:1>
Django
Reporter: heikkimattila | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+--------------------------------------
Comment (by Simon Charette):
Right, this should have been fixed by #27699 which is included in 1.11.x.
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:2>
Django
Reporter: heikkimattila | Owner: nobody
Type: Bug | Status: closed
Component: Utilities | Version: 1.11
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+--------------------------------------
Changes (by Tim Graham):
* status: new => closed
* resolution: => needsinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:3>
Django
Reporter: heikkimattila | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+--------------------------------------
* status: closed => new
* resolution: needsinfo =>
Comment:
Example cases, can be discussed:
parse_duration('-00:01:01') => plus 61 seconds, so it is not -(00:01:01)
but (-00):(+01):(+01)
**parse_duration('00:-01:-01) => None , leading zeros will prevent
parsing**
parse_duration('-01:01') => minus 59 seconds
parse_duration('-01:-01') => minus 61 seconds
The fix presented would allow the second line to be parsed (which would
help with generated durations).
And some instructions in the function/documentation/wiki would be useful,
to clarify how the minus sign affects in duration.
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:4>
Django
-------------------------------+------------------------------------
Reporter: heikkimattila | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+------------------------------------
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
The fix from #27699 may not be entirely correct. I agree with your first
and third examples. I'd expect a leading minus sign to negate the entire
value so they would be minus 61 seconds. I think the second and fourth
examples are invalid. I don't think a minus sign after a colon is valid.
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:5>
Django
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+------------------------------------
Comment (by Simon Charette):
Thanks for the extra details. I agree with Tim that everything but a
leading `-` seems like an invalid value that happened to work because of
an inappropriate pattern as
[https://github.com/django/django/pull/7814/files#diff-
3de806c0e75415eef08ee4268d3efd70R111 it was never tested].
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:6>
Django
Reporter: heikkimattila | Owner: Seunghun Lee
Type: Bug | Status: assigned
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+----------------------------------------
Changes (by Seunghun Lee):
* owner: nobody => Seunghun Lee
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:7>
Django
-------------------------------+----------------------------------------
Reporter: heikkimattila | Owner: Seunghun Lee
Type: Bug | Status: assigned
Component: Utilities | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/10999 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:8>
Django
-------------------------------+----------------------------------------
Reporter: heikkimattila | Owner: Seunghun Lee
Component: Utilities | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"99fc5dc13c12d874ffc1c8f47a6421494e720b31" 99fc5dc]:
{{{
#!CommitTicketReference repository=""
revision="99fc5dc13c12d874ffc1c8f47a6421494e720b31"
Fixed #30141 -- Fixed parse_duration() for some negative durations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:10>
Django
-------------------------------+----------------------------------------
Reporter: heikkimattila | Owner: Seunghun Lee
Type: Bug | Status: closed
Component: Utilities | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"99fc5dc13c12d874ffc1c8f47a6421494e720b31" 99fc5dc]:
{{{
#!CommitTicketReference repository=""
revision="99fc5dc13c12d874ffc1c8f47a6421494e720b31"
Fixed #30141 -- Fixed parse_duration() for some negative durations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30141#comment:9>