DurationField, request for help and comments
Adys
I've been working on DurationFields this evening (ticket #2443)[1]. My
implementation is working pretty well with a single TextInput. I'm
storing the durations as Decimal in the database (since from what I've
heard storing them as a 64-bit int is a no-go), in microseconds.
When creating or modifying a DurationField, the user can either use
ints/floats/float strings which will be interpreted as seconds, or use
a datetime.timedelta object.
In the admin (this will need some inline help or similar), users have
a single text input, which lets them type durations as, for example
"1w" for 1 week, "1h 30min", or even "1y 7m 6w 3d 18h 30min 23s 10ms
150mis" (in order of all accepted values). The values don't have to be
in a specific order and a repeated one will be accounted both times
(10d 1h 5d will be 15d 1h).
What I need help with:
- The admin output is the result of a str(timedelta), I haven't
figured out how to use from_timedelta() on the value; so for now a
DurationField will show "10 days, 0:00:00" when it should show "10d".
- Moving from_timedelta, to_timedelta and values_in_milliseconds
elsewhere. They are reusable for other similar fields and don't belong
where I coded them. But I'm not familiar with Django, and I don't know
where they should be. Somewhere in django.utils?
- Any unnecessary code I haven't spotted. My patch is based on the
previous DurationField implementation which dates from pre 1.0, I'm
unsure if there's anything not necessary anymore.
- Any comment on the code, the implementation, etc is appreciated.
However I haven't sent other Django patches before, be nice :-)
Thanks.
Jerome
[1] The patch: http://code.djangoproject.com/attachment/ticket/2443/durationfield-new.diff
Russell Keith-Magee
>
> I've been working on DurationFields this evening (ticket #2443)[1]. My
Hi Jerome,
Most of the people that could give you the feedback you are requesting
are currently working hard to get version 1.1 out the door. If you
wait a couple of weeks for the dust to settle on v1.1, you may find
you get a better response to your requests for review and assistance.
Yours,
Russ Magee %-)
Yuri Baburov
I solved this problem, wrapping datetime.timedelta into custom TimeDelta.
Now everything works: http://gist.github.com/117307
Also altered microseconds in one month value so 12 * month are equal to 1 year.
This fixes "13m" bug (breaking into smaller parts incl. microseconds).
"31d" bug is still there.
Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov,
MSN: bu...@live.com
Jerome Leclanche
- Why mention *args at all (l20,21) if it's to assert they don't exist?
- Any idea where to put the TimeDelta class? I still don't believe it
belongs in django.widgets, but I'm not familiar enough with the
structure to know where to put it.
Cheers, I'll try the patch tomorrow.
Jerome
Adys
Jerome Leclanche
J
Adys
Yuri Baburov
On Mon, May 25, 2009 at 9:27 AM, Jerome Leclanche <ady...@gmail.com> wrote:
>
> Hi Yuri, thanks for the fixes. Two questions:
>
> - Why mention *args at all (l20,21) if it's to assert they don't exist?
Just to show friendlier user message.
Without args TimeDelta(x) is TimeDelta(days=x) so it tells that what
you passed in can't be in day component.
This was not very clean solution, I just hadn't time to figure out the best way.
datetime.timedelta __new__ method shall have the following signature:
def datetime.timedelta(days=0, seconds=0, microseconds=0): pass
We need to acquire data from "12h 30min" and Decimal("7260000") and
datetime.timedelta and "0" in the most pretty way. Of course the
prettiest is TimeDelta(x).
But if data is days, and is int, say "5", we should let existing code work.
I didn't manage to solve this puzzle in those 5 minutes I had for it.
TimeDelta(from=None, days=0, seconds=0, microseconds=0)
will break days handling if anyone will change the datetime.timedelta
into TimeDelta.
So, the best I found was something like
TimeDelta(*args, days=0, seconds=0, microseconds=0) and processing
args by myself.
But since I hadn't much time, I asserted *args to make sure I'm not
using TimeDelta().
That's how I forgot about days handling idea with TimeDelta(5, 3, 1).
After few days passed, I tend to think the best is the most explicit:
TimeDelta(days=0, seconds=0, microseconds=0, from=None)
Changing gist now.
> - Any idea where to put the TimeDelta class? I still don't believe it
> belongs in django.widgets, but I'm not familiar enough with the
> structure to know where to put it.
Somewhere in django.utils, I believe, but from_timetuple method should
move into constructor too, and a single line should stay in
from_timetuple.
> Cheers, I'll try the patch tomorrow.
>
> Jerome
>
> On Mon, May 25, 2009 at 4:12 AM, Yuri Baburov <bur...@gmail.com> wrote:
>>
>> Hi Adys,
>>
>> I solved this problem, wrapping datetime.timedelta into custom TimeDelta.
>> Now everything works: http://gist.github.com/117307
>>
>> Also altered microseconds in one month value so 12 * month are equal to 1 year.
>> This fixes "13m" bug (breaking into smaller parts incl. microseconds).
>> "31d" bug is still there.
Any idea if "31d" bug should be fixed?
Yuri Baburov
On Mon, May 25, 2009 at 9:27 AM, Jerome Leclanche <ady...@gmail.com> wrote:
>
> Hi Yuri, thanks for the fixes. Two questions:
>
> - Why mention *args at all (l20,21) if it's to assert they don't exist?
Just to show friendlier user message.
Without args TimeDelta(x) is TimeDelta(days=x) so it tells that what
you passed in can't be in day component.
This was not very clean solution, I just hadn't time to figure out the best way.
datetime.timedelta __new__ method shall have the following signature:
def datetime.timedelta(days=0, seconds=0, microseconds=0): pass
We need to acquire data from "12h 30min" and Decimal("7260000") and
datetime.timedelta and "0" in the most pretty way. Of course the
prettiest is TimeDelta(x).
But if data is days, and is int, say "5", we should let existing code work.
I didn't manage to solve this puzzle in those 5 minutes I had for it.
TimeDelta(from=None, days=0, seconds=0, microseconds=0)
will break days handling if anyone will change the datetime.timedelta
into TimeDelta.
So, the best I found was something like
TimeDelta(*args, days=0, seconds=0, microseconds=0) and processing
args by myself.
But since I hadn't much time, I asserted *args to make sure I'm not
using TimeDelta().
That's how I forgot about days handling idea with TimeDelta(5, 3, 1).
After few days passed, I tend to think the best is the most explicit:
TimeDelta(days=0, seconds=0, microseconds=0, from=None)
Changing gist now.
> - Any idea where to put the TimeDelta class? I still don't believe it
> belongs in django.widgets, but I'm not familiar enough with the
> structure to know where to put it.
Somewhere in django.utils, I believe, but from_timetuple method should
move into constructor too, and a single line should stay in
from_timetuple.
> Cheers, I'll try the patch tomorrow.
>
> Jerome
>
> On Mon, May 25, 2009 at 4:12 AM, Yuri Baburov <bur...@gmail.com> wrote:
>>
>> Hi Adys,
>>
>> I solved this problem, wrapping datetime.timedelta into custom TimeDelta.
>> Now everything works: http://gist.github.com/117307
>>
>> Also altered microseconds in one month value so 12 * month are equal to 1 year.
>> This fixes "13m" bug (breaking into smaller parts incl. microseconds).
>> "31d" bug is still there.
Any idea if "31d" bug should be fixed?
--
Jerome Leclanche
value of "1m 13h 30min 56s 167ms 200us", which is the correct
duration. (=> http://www.google.com/search?q=how+many+days+in+a+month
).
The problem of breaking into smaller parts is that a month isn't an
integer amount of days; if it needs fixing, it should probably check
if the value is equal to an integer amount of months/years first, then
check if it's equal to an integer amount of days:
- Yes / (No|Yes): Display as month/year
- No / No: Break into parts (current behaviour)
- No / Yes: Display as days
Another thing I noticed this morning: When using a DurationField as
filter into the admin, I get values in microseconds rather than the
"prettified" version. e.g.:
By duration
* All
* 0
* 20000000
* 30000000
* 60000000
* 180000000
* ...
I updated the patch on ticket #2443 earlier today to apply cleanly to
current trunk. It's the one I've been using today and I haven't found
any other issue.
Adys
Marty Alchin
to say I'm not thrilled with the direction it's currently headed. I've
never liked the "1h 30m 10s" syntax in any situation I've ever
encountered it, and I highly doubt that it will be meaningful to much
of anyone. That's personal opinion, of course, but I just don't see it
being a very good solution. My preferred approach would be to simply
separate the days from everything else and let two fields handle it. A
day field would parse an integer, while a time field would parse
standard time syntax ("01:30:10"), which is much more commonly used
for durations.
I'll freely admit that this isn't perfectly intuitive either, but I
think it's much more natural than the current proposal. I have a
slightly modified version of my original patch that works a bit better
now, and I'm working on a JavaScript helper like the current TimeField
uses in the admin, which should help relieve the intuitiveness
problem.
More specifically, can someone explain to me just what problems are
solved by adding a custom TimeDelta class? I can only see two benefits
to it: it accepts years and months as units and it maps the new syntax
into meaningful values. I've already spoken on the syntax issue, but
I'd like to point out that accepting years and months is a very bad
idea.
Basically, there are two ways to deal with years and months, depending
on what situation you're trying to use them in. The first is to not
try to flatten it out to a specific value until applied to a reference
date. This is the approach used by calendar applications, for
instance, when you apply a yearly repeating rule with a start date or
set an alarm for one month in advance of a given event. There are
known reference dates to work with, so years and months can be
calculated accurately.
The other approach is to assign some approximate value to years and
months, which is especially useful when *not* dealing with reference
dates. For example, you might use this approach to describe a
university degree program that takes four years or a prison sentence
that lasts 6 months. When speaking in generic terms, these
approximations are appropriate, but only because they're abstract in
our minds. They're never given an absolute length of time that can be
collapsed to days, hours, minutes or seconds. That can only happen
once a true date can be assigned.
What this ticket is doing, however, is trying to formalize the second
approach by explicitly stating some approximate values for years and
months. The intent is admirable, but it's ultimately doomed. What
you're left with is a "solution" that passably works for the second
situation, but outright fails for the first. When a custom TimeDelta
with years and months is applied to a known date, the result will
nearly always be incorrect. All it really does is formalize something
that shouldn't be formalized anyway.
The third option, which is used by Python and many other computing
systems, is to take years and months out of the equation entirely.
Sure, it's a reduction in functionality, but the benefit is that what
remains *actually works*. Weeks are the largest unit of time that can
be accurately collapsed to seconds on its own and applied to date
calculations in all situations. Thus, Python's timedelta supports
weeks as the largest available unit.
I saw this issue raised on the ticket, with the response of, "I just
wanted to get the implementation in. It's easier to remove than to
add-on." Unfortunately, there have been two more patches from the same
author and the year and month support remains. I hope I've explained
the situation well enough now that we can forget about years and
months in future patches.
I'll address the question of input format once I have a new patch up
and running.
-Gul
Jerome Leclanche
Right now, removing months and years is a matter of removing two lines
from the patch. I didn't want to write a patch just to get told "it's
lacking months/years!"; I'm also in favour of dropping it and
following Python's style; but up until now no one except yourself and
Yuri have given feedback, I had no reason to drop it.
As for the current syntax, I believe with a small syntax helper
somewhere next to the field, this would be and feel completely
natural. 01:30:10 is a bit meaningless if you're not working in
seconds. One of my application uses standard milliseconds durations,
would I have to write 01:30:10.5 to have another 500ms? Would I need
1-2 other fields for milli/microseconds - which would make us go back
to the former problem ("Many input boxes is not a nice user
experience")?
That's the way I was going until someone came up with the idea of
parsing a timestring. Looking back, this is much more elegant, as long
as the user knows what to do. The need for a javascript helper is
entirely dropped since it's extremely easy to input any kind of value,
and the user doesn't have to convert any kind of value. If the user
wants 17 weeks, they just input "17w" instead of having to calculate
119 days. If they want "1500 seconds", same thing; so on.
Also remember not everyone has javascript enabled.
As for a wrapper TimeDelta class, that's up to Yuri to explain - I
don't see the problem with it personally.
Cheers,
Jerome L.
Adys
J. Leclanche
Marty Alchin
> Right now, removing months and years is a matter of removing two lines
> from the patch. I didn't want to write a patch just to get told "it's
> lacking months/years!"; I'm also in favour of dropping it and
> following Python's style; but up until now no one except yourself and
> Yuri have given feedback, I had no reason to drop it.
My only beef here is that Yuri did mention it, with reasonable
justification in the ticket, and you did acknowledge that it could be
removed, yet it remained. I can understand the tendency to support as
much as possible until someone reigns you in, but once someone does,
it's best to listen. But, I've spoken my peace and it sounds like
we're on the same page now, so we move on.
> As for the current syntax, I believe with a small syntax helper
> somewhere next to the field, this would be and feel completely
> natural. 01:30:10 is a bit meaningless if you're not working in
> seconds. One of my application uses standard milliseconds durations,
> would I have to write 01:30:10.5 to have another 500ms? Would I need
> 1-2 other fields for milli/microseconds - which would make us go back
> to the former problem ("Many input boxes is not a nice user
> experience")?
In my view, yes, 01:30:10.5 (or 01:30:10.500000 if you prefer) would
be the better approach. I will, however, freely admit that my support
for that syntax is based on nothing more than personal preference and
a vague idea of what I believe to be more common. I don't have any
research to back it up, but I don't expect any of us do in this area.
A quick search reveals that ISO 8601 is the relevant specification
here, which would leave us with something along the lines of
PT01H30M10S. In addition to being not-so-user-friendly, it seems ISO
8601 doesn't provide for anything smaller than a second, so we're back
to that same problem anyway.
> That's the way I was going until someone came up with the idea of
> parsing a timestring. Looking back, this is much more elegant, as long
> as the user knows what to do. The need for a javascript helper is
> entirely dropped since it's extremely easy to input any kind of value,
> and the user doesn't have to convert any kind of value. If the user
> wants 17 weeks, they just input "17w" instead of having to calculate
> 119 days. If they want "1500 seconds", same thing; so on.
I'll admit, my preferred approach does drop support for weeks, and if
you were to put in more than 24 hours, 60 minutes or 60 seconds, you'd
get a big, fat error message instead of the implicit calculations that
timedelta does behind the scenes. I still disagree with the notion
that the current syntax is "extremely easy to input any kind of
value," but again, neither statement is based on very much, so it's
likely impossible to say which is "better".
We're basically debating between the two representations allowed by
ISO 8601, at least in spirit. It seems an international panel of
experts couldn't decide on one over the other either, so they allow
both. Unfortunately, that's not really in keeping with Python
philosophy ("There should be one-- and preferably only one --obvious
way to do it."), so we need to pick one or the other. Whichever way we
go, it seems some of us will be Dutch, some of us won't. Such is life.
At this point, I'd say we should table the discussion on input syntax
until after 1.1 is out the door, so we can get some input from people
who have more sway in these matters. And yes, I do realize that I'm
the one who brought it up. ;)
> Also remember not everyone has javascript enabled.
I've seen this argument before, and it doesn't hold water. A
JavaScript helper is just that: a helper. It's not required to use the
field, it just makes it a bit easier for those who need a little help.
Those without JavaScript would just get text boxes without the helper,
just like the existing date and time fields.
> As for a wrapper TimeDelta class, that's up to Yuri to explain - I
> don't see the problem with it personally.
I wasn't pointing fingers at anybody in particular, I just don't see
that it adds anything beyond Python's own datetime.timedelta. Once
years and months are removed, it basically just performs unit
conversion and provides an output format in line with the new input
syntax. Python's timedelta already does the unit conversion, so that's
just duplicated effort. As for the output format, I'd rather see that
handled at the widget level anyway, so any other Python code that
deals with timedeltas doesn't get a shock when the output isn't what
Python normally provides.
-Gul
Yuri Baburov
I saw such widget in time planning apps (Jira bugtracker) and like it very much.
When I need to tell that I will work "4 hours" on task, i don't want
to see [ 0 ] days [ 4:00:00 ] seconds, and when I tell milestone will
long for 6 weeks, i want to see just "1 month 2 weeks", not "[ 42 ]
days [ 0:00:00.000 ] seconds", not even "1m 1w 4d 13h 30min 56s 167ms
200us" as it's now.
You see, I have a very practical use case too.
Well, I agree that in admin the more formal widget, that Marti
suggests, might suit better.
As an end user of this functionality, I will be happy iff I will be
able to use TimeDelta representation and widget into a field, and
change year duration to one that's comfortable to me.